Make Bruker converters add_to_nwb and run_conversion consistent with all the other converters

CHANGELOG.md

@@ -2,6 +2,7 @@
 
 ### Bug fixes
 * Fixed the default naming of multiple electrical series in the `SpikeGLXConverterPipe`. [PR #957](https://github.com/catalystneuro/neuroconv/pull/957)
+* Fixed an issue when passing conversion options to a sub-converter (like the popular `SpikeGLXConverterPipe`) nested inside another `NWBConverter`. [PR #979](https://github.com/catalystneuro/neuroconv/pull/979)
 
 ### Improvements
 * The `OpenEphysBinaryRecordingInterface` now uses `lxml` for extracting the session start time from the settings.xml file and does not depend on `pyopenephys` anymore. [PR #971](https://github.com/catalystneuro/neuroconv/pull/971)

src/neuroconv/datainterfaces/ophys/brukertiff/brukertiffconverter.py

@@ -1,3 +1,6 @@
+# TODO: Remove add_to_nwbfile and run_conversion and let inheritance handle it
+
+import warnings
 from typing import Literal, Optional
 
 from pynwb import NWBFile
@@ -83,16 +86,39 @@ def add_to_nwbfile(
         self,
         nwbfile: NWBFile,
         metadata,
-        stub_test: bool = False,
-        stub_frames: int = 100,
+        stub_test: Optional[bool] = False,
+        stub_frames: Optional[int] = 100,
+        conversion_options: Optional[dict] = None,
     ):
+
+        # Put deprecation warnings for passing stub_test and stub_frames directly
+        if stub_test is not None:
+            warnings.warn(
+                "The 'stub_test' argument is deprecated and will be removed at some point after February 2025"
+                "Please set 'stub_test' during the initialization of the BrukerTiffMultiPlaneConverter instance.",
+                DeprecationWarning,
+                stacklevel=2,
+            )
+
+        if stub_frames is not None:
+            warnings.warn(
+                "The 'stub_frames' argument is deprecated and will be removed at some point after February 2025"
+                "Please set 'stub_frames' during the initialization of the BrukerTiffMultiPlaneConverter instance.",
+                DeprecationWarning,
+                stacklevel=2,
+            )
+
+        conversion_options = conversion_options or dict()
         for photon_series_index, (interface_name, data_interface) in enumerate(self.data_interface_objects.items()):
+            interface_conversion_options = conversion_options.get(interface_name, dict())
+            interface_conversion_options["stub_test"] = stub_test
+            interface_conversion_options["stub_frames"] = stub_frames
+
             data_interface.add_to_nwbfile(
                 nwbfile=nwbfile,
                 metadata=metadata,
                 photon_series_index=photon_series_index,
-                stub_test=stub_test,
-                stub_frames=stub_frames,
+                **interface_conversion_options,
             )
 
     def run_conversion(
@@ -103,7 +129,26 @@ def run_conversion(
         overwrite: bool = False,
         stub_test: bool = False,
         stub_frames: int = 100,
+        conversion_options: Optional[dict] = None,
     ) -> None:
+
+        # Put deprecation warnings for passing stub_test and stub_frames directly
+        if stub_test is not None:
+            warnings.warn(
+                "The 'stub_test' argument is deprecated and will be removed at some point after February 2025"
+                "Please set 'stub_test' during the initialization of the BrukerTiffMultiPlaneConverter instance.",
+                DeprecationWarning,
+                stacklevel=2,
+            )
+
+        if stub_frames is not None:
+            warnings.warn(
+                "The 'stub_frames' argument is deprecated and will be removed at some point after February 2025"
+                "Please set 'stub_frames' during the initialization of the BrukerTiffMultiPlaneConverter instance.",
+                DeprecationWarning,
+                stacklevel=2,
+            )
+
         if metadata is None:
             metadata = self.get_metadata()
 
@@ -118,7 +163,13 @@ def run_conversion(
             overwrite=overwrite,
             verbose=self.verbose,
         ) as nwbfile_out:
-            self.add_to_nwbfile(nwbfile=nwbfile_out, metadata=metadata, stub_test=stub_test, stub_frames=stub_frames)
+            self.add_to_nwbfile(
+                nwbfile=nwbfile_out,
+                metadata=metadata,
+                stub_test=stub_test,
+                stub_frames=stub_frames,
+                conversion_options=conversion_options,
+            )
 
 
 class BrukerTiffSinglePlaneConverter(NWBConverter):
@@ -175,16 +226,39 @@ def add_to_nwbfile(
         self,
         nwbfile: NWBFile,
         metadata,
-        stub_test: bool = False,
-        stub_frames: int = 100,
+        stub_test: Optional[bool] = False,
+        stub_frames: Optional[int] = 100,
+        conversion_options: Optional[dict] = None,
     ):
+
+        # Put deprecation warnings for passing stub_test and stub_frames directly
+        if stub_test is not None:
+            warnings.warn(
+                "The 'stub_test' argument is deprecated and will be removed at some point after February 2025"
+                "Please set 'stub_test' during the initialization of the BrukerTiffMultiPlaneConverter instance.",
+                DeprecationWarning,
+                stacklevel=2,
+            )
+
+        if stub_frames is not None:
+            warnings.warn(
+                "The 'stub_frames' argument is deprecated and will be removed at some point after February 2025"
+                "Please set 'stub_frames' during the initialization of the BrukerTiffMultiPlaneConverter instance.",
+                DeprecationWarning,
+                stacklevel=2,
+            )
+
+        conversion_options = conversion_options or dict()
         for photon_series_index, (interface_name, data_interface) in enumerate(self.data_interface_objects.items()):
+            interface_conversion_options = conversion_options.get(interface_name, dict())
+            interface_conversion_options["stub_test"] = stub_test
+            interface_conversion_options["stub_frames"] = stub_frames
+
             data_interface.add_to_nwbfile(
                 nwbfile=nwbfile,
                 metadata=metadata,
                 photon_series_index=photon_series_index,
-                stub_test=stub_test,
-                stub_frames=stub_frames,
+                **interface_conversion_options,
             )
 
     def run_conversion(
@@ -193,9 +267,28 @@ def run_conversion(
         nwbfile: Optional[NWBFile] = None,
         metadata: Optional[dict] = None,
         overwrite: bool = False,
-        stub_test: bool = False,
-        stub_frames: int = 100,
+        stub_test: Optional[bool] = False,
+        stub_frames: Optional[int] = 100,
+        conversion_options: Optional[dict] = None,
     ) -> None:
+
+        # Put deprecation warnings for passing stub_test and stub_frames directly
+        if stub_test is not None:
+            warnings.warn(
+                "The 'stub_test' argument is deprecated and will be removed at some point after February 2025"
+                "Please set 'stub_test' during the initialization of the BrukerTiffMultiPlaneConverter instance.",
+                DeprecationWarning,
+                stacklevel=2,
+            )
+
+        if stub_frames is not None:
+            warnings.warn(
+                "The 'stub_frames' argument is deprecated and will be removed at some point after February 2025"
+                "Please set 'stub_frames' during the initialization of the BrukerTiffMultiPlaneConverter instance.",
+                DeprecationWarning,
+                stacklevel=2,
+            )
+
         if metadata is None:
             metadata = self.get_metadata()
 
@@ -210,4 +303,10 @@ def run_conversion(
             overwrite=overwrite,
             verbose=self.verbose,
         ) as nwbfile_out:
-            self.add_to_nwbfile(nwbfile=nwbfile_out, metadata=metadata, stub_test=stub_test, stub_frames=stub_frames)
+            self.add_to_nwbfile(
+                nwbfile=nwbfile_out,
+                metadata=metadata,
+                stub_test=stub_test,
+                stub_frames=stub_frames,
+                conversion_options=conversion_options,
+            )

src/neuroconv/nwbconverter.py

@@ -158,9 +158,34 @@ def create_nwbfile(self, metadata: Optional[dict] = None, conversion_options: Op
 
     def add_to_nwbfile(self, nwbfile: NWBFile, metadata, conversion_options: Optional[dict] = None) -> None:
         conversion_options = conversion_options or dict()
-        for interface_name, data_interface in self.data_interface_objects.items():
+
+        sub_objects = self.data_interface_objects
+        data_interfaces = {
+            interface_key: interface
+            for interface_key, interface in sub_objects.items()
+            if isinstance(interface, BaseDataInterface)
+        }
+
+        for interface_key, data_interface in data_interfaces.items():
+            interface_conversion_options = conversion_options.get(interface_key, dict())
             data_interface.add_to_nwbfile(
-                nwbfile=nwbfile, metadata=metadata, **conversion_options.get(interface_name, dict())
+                nwbfile=nwbfile,
+                metadata=metadata,
+                **interface_conversion_options,
+            )
+
+        converters = {
+            converter_key: converter
+            for converter_key, converter in sub_objects.items()
+            if isinstance(converter, NWBConverter)
+        }
+
+        for converter_key, converter in converters.items():
+            converter_conversion_options_dict = conversion_options.get(converter_key, dict())
+            converter.add_to_nwbfile(
+                nwbfile=nwbfile,
+                metadata=metadata,
+                conversion_options=converter_conversion_options_dict,
             )
 
     def run_conversion(

tests/test_on_data/test_format_converters/test_spikeglx_converter.py

@@ -96,6 +96,30 @@ class TestConverter(NWBConverter):
         expected_session_start_time = datetime(2020, 11, 3, 10, 35, 10).astimezone()
         self.assertNWBFileStructure(nwbfile_path=nwbfile_path, expected_session_start_time=expected_session_start_time)
 
+    def test_single_probe_spikeglx_converter_in_converter_preview(self):
+        class TestConverter(NWBConverter):
+            data_interface_classes = dict(SpikeGLX=SpikeGLXConverterPipe)
+
+        source_data = dict(SpikeGLX=dict(folder_path=str(SPIKEGLX_PATH / "Noise4Sam_g0")))
+        converter = TestConverter(source_data=source_data)
+
+        conversion_options = dict(
+            SpikeGLX={"imec0.ap": {"stub_test": True}, "imec0.lf": {"stub_test": True}, "nidq": {"stub_test": True}}
+        )
+
+        nwbfile_path = self.tmpdir / "test_single_probe_spikeglx_converter_preview.nwb"
+        converter.run_conversion(nwbfile_path=nwbfile_path, conversion_options=conversion_options)
+
+        expected_session_start_time = datetime(2020, 11, 3, 10, 35, 10).astimezone()
+        self.assertNWBFileStructure(nwbfile_path=nwbfile_path, expected_session_start_time=expected_session_start_time)
+
+        with NWBHDF5IO(path=nwbfile_path) as io:
+            nwbfile = io.read()
+
+            assert nwbfile.acquisition["ElectricalSeriesAPImec0"].data.shape == (100, 384)
+            assert nwbfile.acquisition["ElectricalSeriesLFImec0"].data.shape == (100, 384)
+            assert nwbfile.acquisition["ElectricalSeriesNIDQ"].data.shape == (100, 9)
+
 
 class TestMultiProbeSpikeGLXConverter(TestCase):
     maxDiff = None