From f2e90317abdb84478f611b110ee5ca31c63d6ff1 Mon Sep 17 00:00:00 2001 From: Heberto Mayorquin Date: Fri, 13 Sep 2024 18:25:39 -0600 Subject: [PATCH 1/4] more zarr testing to data --- src/neuroconv/basedatainterface.py | 11 ++-- .../tools/testing/data_interface_mixins.py | 62 +++++++++---------- .../ecephys/test_recording_interfaces.py | 13 ++++ 3 files changed, 45 insertions(+), 41 deletions(-) diff --git a/src/neuroconv/basedatainterface.py b/src/neuroconv/basedatainterface.py index 59415b043..682ed2dba 100644 --- a/src/neuroconv/basedatainterface.py +++ b/src/neuroconv/basedatainterface.py @@ -127,11 +127,8 @@ def run_conversion( nwbfile: Optional[NWBFile] = None, metadata: Optional[dict] = None, overwrite: bool = False, - # TODO: when all H5DataIO prewraps are gone, introduce Zarr safely - # backend: Union[Literal["hdf5", "zarr"]], - # backend_configuration: Optional[Union[HDF5BackendConfiguration, ZarrBackendConfiguration]] = None, - backend: Optional[Literal["hdf5"]] = None, - backend_configuration: Optional[HDF5BackendConfiguration] = None, + backend: Optional[Literal["hdf5", "zarr"]] = None, + backend_configuration: Optional[Union[HDF5BackendConfiguration, ZarrBackendConfiguration]] = None, **conversion_options, ): """ @@ -148,11 +145,11 @@ def run_conversion( overwrite : bool, default: False Whether to overwrite the NWBFile if one exists at the nwbfile_path. The default is False (append mode). - backend : "hdf5", optional + backend : {"hdf5", "zarr"}, optional The type of backend to use when writing the file. If a `backend_configuration` is not specified, the default type will be "hdf5". If a `backend_configuration` is specified, then the type will be auto-detected. - backend_configuration : HDF5BackendConfiguration, optional + backend_configuration : HDF5BackendConfiguration or ZarrBackendConfiguration, optional The configuration model to use when configuring the datasets for this backend. To customize, call the `.get_default_backend_configuration(...)` method, modify the returned BackendConfiguration object, and pass that instead. diff --git a/src/neuroconv/tools/testing/data_interface_mixins.py b/src/neuroconv/tools/testing/data_interface_mixins.py index 2b8252154..473e89e33 100644 --- a/src/neuroconv/tools/testing/data_interface_mixins.py +++ b/src/neuroconv/tools/testing/data_interface_mixins.py @@ -106,32 +106,6 @@ def test_no_metadata_mutation(self, setup_interface): self.interface.add_to_nwbfile(nwbfile=nwbfile, metadata=metadata, **self.conversion_options) - assert metadata == metadata_before_add_method - - def check_run_conversion_with_backend(self, nwbfile_path: str, backend: Literal["hdf5", "zarr"] = "hdf5"): - metadata = self.interface.get_metadata() - if "session_start_time" not in metadata["NWBFile"]: - metadata["NWBFile"].update(session_start_time=datetime.now().astimezone()) - - self.interface.run_conversion( - nwbfile_path=nwbfile_path, - overwrite=True, - metadata=metadata, - backend=backend, - **self.conversion_options, - ) - - def check_configure_backend_for_equivalent_nwbfiles(self, backend: Literal["hdf5", "zarr"] = "hdf5"): - metadata = self.interface.get_metadata() - if "session_start_time" not in metadata["NWBFile"]: - metadata["NWBFile"].update(session_start_time=datetime.now().astimezone()) - - nwbfile_1 = self.interface.create_nwbfile(metadata=metadata, **self.conversion_options) - nwbfile_2 = self.interface.create_nwbfile(metadata=metadata, **self.conversion_options) - - backend_configuration = get_default_backend_configuration(nwbfile=nwbfile_1, backend=backend) - configure_backend(nwbfile=nwbfile_2, backend_configuration=backend_configuration) - def check_run_conversion_with_backend_configuration( self, nwbfile_path: str, backend: Literal["hdf5", "zarr"] = "hdf5" ): @@ -204,11 +178,6 @@ def check_read_nwb(self, nwbfile_path: str): """Read the produced NWB file and compare it to the interface.""" pass - def check_basic_zarr_read(self, nwbfile_path: str): - """Ensure NWBZarrIO can read the file.""" - with NWBZarrIO(path=nwbfile_path, mode="r") as io: - io.read() - def check_extracted_metadata(self, metadata: dict): """Override this method to make assertions about specific extracted metadata values.""" pass @@ -235,7 +204,20 @@ def test_run_conversion_with_backend(self, setup_interface, tmp_path, backend): ) if backend == "zarr": - self.check_basic_zarr_read(nwbfile_path) + with NWBZarrIO(path=nwbfile_path, mode="r") as io: + io.read() + + @pytest.mark.parametrize("backend", ["hdf5", "zarr"]) + def test_configure_backend_for_equivalent_nwbfiles(self, setup_interface, tmp_path, backend): + metadata = self.interface.get_metadata() + if "session_start_time" not in metadata["NWBFile"]: + metadata["NWBFile"].update(session_start_time=datetime.now().astimezone()) + + nwbfile_1 = self.interface.create_nwbfile(metadata=metadata, **self.conversion_options) + nwbfile_2 = self.interface.create_nwbfile(metadata=metadata, **self.conversion_options) + + backend_configuration = get_default_backend_configuration(nwbfile=nwbfile_1, backend=backend) + configure_backend(nwbfile=nwbfile_2, backend_configuration=backend_configuration) def test_all_conversion_checks(self, setup_interface, tmp_path): interface, test_name = setup_interface @@ -247,7 +229,6 @@ def test_all_conversion_checks(self, setup_interface, tmp_path): # Now run the checks using the setup objects self.check_conversion_options_schema_valid() self.check_metadata() - self.check_configure_backend_for_equivalent_nwbfiles() self.check_run_conversion_in_nwbconverter_with_backend(nwbfile_path=nwbfile_path, backend="hdf5") self.check_run_conversion_in_nwbconverter_with_backend_configuration(nwbfile_path=nwbfile_path, backend="hdf5") @@ -746,7 +727,6 @@ def test_all_conversion_checks(self, setup_interface, tmp_path): # Now run the checks using the setup objects self.check_conversion_options_schema_valid() self.check_metadata() - self.check_configure_backend_for_equivalent_nwbfiles() self.check_run_conversion_in_nwbconverter_with_backend(nwbfile_path=nwbfile_path, backend="hdf5") self.check_run_conversion_in_nwbconverter_with_backend_configuration(nwbfile_path=nwbfile_path, backend="hdf5") @@ -1000,6 +980,17 @@ class TestNWBConverter(NWBConverter): conversion_options=conversion_options, ) + def check_configure_backend_for_equivalent_nwbfiles(self, backend: Literal["hdf5", "zarr"] = "hdf5"): + metadata = self.interface.get_metadata() + if "session_start_time" not in metadata["NWBFile"]: + metadata["NWBFile"].update(session_start_time=datetime.now().astimezone()) + + nwbfile_1 = self.interface.create_nwbfile(metadata=metadata, **self.conversion_options) + nwbfile_2 = self.interface.create_nwbfile(metadata=metadata, **self.conversion_options) + + backend_configuration = get_default_backend_configuration(nwbfile=nwbfile_1, backend=backend) + configure_backend(nwbfile=nwbfile_2, backend_configuration=backend_configuration) + def test_all_conversion_checks(self, metadata: dict): interface_kwargs = self.interface_kwargs if isinstance(interface_kwargs, dict): @@ -1263,6 +1254,9 @@ def test_run_conversion_with_backend(self): def test_no_metadata_mutation(self): pass + def test_configure_backend_for_equivalent_nwbfiles(self): + pass + def check_metadata_schema_valid(self): schema = self.interface.get_metadata_schema() Draft7Validator.check_schema(schema=schema) diff --git a/tests/test_on_data/ecephys/test_recording_interfaces.py b/tests/test_on_data/ecephys/test_recording_interfaces.py index cc83625dc..421321796 100644 --- a/tests/test_on_data/ecephys/test_recording_interfaces.py +++ b/tests/test_on_data/ecephys/test_recording_interfaces.py @@ -179,6 +179,19 @@ class TestEDFRecordingInterface(RecordingExtractorInterfaceTestMixin): def check_extracted_metadata(self, metadata: dict): assert metadata["NWBFile"]["session_start_time"] == datetime(2022, 3, 2, 10, 42, 19) + def check_run_conversion_with_backend(self, nwbfile_path: str, backend="hdf5"): + metadata = self.interface.get_metadata() + if "session_start_time" not in metadata["NWBFile"]: + metadata["NWBFile"].update(session_start_time=datetime.now().astimezone()) + + self.interface.run_conversion( + nwbfile_path=nwbfile_path, + overwrite=True, + metadata=metadata, + backend=backend, + **self.conversion_options, + ) + def test_all_conversion_checks(self, setup_interface, tmp_path): # Create a unique test name and file path nwbfile_path = str(tmp_path / f"{self.__class__.__name__}.nwb") From ae5547c5d91a3b2ee9c558ced7fadbbb883fb13a Mon Sep 17 00:00:00 2001 From: Heberto Mayorquin Date: Fri, 13 Sep 2024 19:39:51 -0600 Subject: [PATCH 2/4] fix edf and med pc as usually --- .../tools/testing/data_interface_mixins.py | 14 +++----------- .../ecephys/test_recording_interfaces.py | 3 +++ 2 files changed, 6 insertions(+), 11 deletions(-) diff --git a/src/neuroconv/tools/testing/data_interface_mixins.py b/src/neuroconv/tools/testing/data_interface_mixins.py index 473e89e33..7763ba8eb 100644 --- a/src/neuroconv/tools/testing/data_interface_mixins.py +++ b/src/neuroconv/tools/testing/data_interface_mixins.py @@ -880,6 +880,9 @@ def test_run_conversion_with_backend(self): def test_no_metadata_mutation(self): pass + def test_configure_backend_for_equivalent_nwbfiles(self): + pass + def check_metadata_schema_valid(self): schema = self.interface.get_metadata_schema() Draft7Validator.check_schema(schema=schema) @@ -980,17 +983,6 @@ class TestNWBConverter(NWBConverter): conversion_options=conversion_options, ) - def check_configure_backend_for_equivalent_nwbfiles(self, backend: Literal["hdf5", "zarr"] = "hdf5"): - metadata = self.interface.get_metadata() - if "session_start_time" not in metadata["NWBFile"]: - metadata["NWBFile"].update(session_start_time=datetime.now().astimezone()) - - nwbfile_1 = self.interface.create_nwbfile(metadata=metadata, **self.conversion_options) - nwbfile_2 = self.interface.create_nwbfile(metadata=metadata, **self.conversion_options) - - backend_configuration = get_default_backend_configuration(nwbfile=nwbfile_1, backend=backend) - configure_backend(nwbfile=nwbfile_2, backend_configuration=backend_configuration) - def test_all_conversion_checks(self, metadata: dict): interface_kwargs = self.interface_kwargs if isinstance(interface_kwargs, dict): diff --git a/tests/test_on_data/ecephys/test_recording_interfaces.py b/tests/test_on_data/ecephys/test_recording_interfaces.py index 421321796..99dab5a8b 100644 --- a/tests/test_on_data/ecephys/test_recording_interfaces.py +++ b/tests/test_on_data/ecephys/test_recording_interfaces.py @@ -218,6 +218,9 @@ def test_run_conversion_with_backend(self): def test_interface_alignment(self): pass + def test_configure_backend_for_equivalent_nwbfiles(self): + pass + class TestIntanRecordingInterfaceRHS(RecordingExtractorInterfaceTestMixin): data_interface_cls = IntanRecordingInterface From f4f011c818fbc4c88ae70ceb4e583a30683350f9 Mon Sep 17 00:00:00 2001 From: Heberto Mayorquin Date: Fri, 13 Sep 2024 19:45:44 -0600 Subject: [PATCH 3/4] CHANGELOG.md --- CHANGELOG.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index dfa9612aa..a59badab2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,10 +11,12 @@ ## Features * Added chunking/compression for string-only compound objects: [PR #1042](https://github.com/catalystneuro/neuroconv/pull/1042) * Added automated EFS volume creation and mounting to the `submit_aws_job` helper function. [PR #1018](https://github.com/catalystneuro/neuroconv/pull/1018) +* Added a mock for segmentation extractors interfaces in ophys: `MockSegmentationInterface` [PR #1067](https://github.com/catalystneuro/neuroconv/pull/1067) ## Improvements * Add writing to zarr test for to the test on data [PR #1056](https://github.com/catalystneuro/neuroconv/pull/1056) -* Modified the CI to avoid running doctests twice [PR #1077](https://github.com/catalystneuro/neuroconv/pull/#1077) +* Modified the CI to avoid running doctests twice +* Added zarr tests for the test on data with checking equivalent backends [PR #1083](https://github.com/catalystneuro/neuroconv/pull/1083) ## v0.6.3 @@ -33,8 +35,6 @@ * Added `get_stream_names` to `OpenEphysRecordingInterface`: [PR #1039](https://github.com/catalystneuro/neuroconv/pull/1039) * Most data interfaces and converters now use Pydantic to validate their inputs, including existence of file and folder paths. [PR #1022](https://github.com/catalystneuro/neuroconv/pull/1022) * All remaining data interfaces and converters now use Pydantic to validate their inputs, including existence of file and folder paths. [PR #1055](https://github.com/catalystneuro/neuroconv/pull/1055) -* Added a mock for segmentation extractors interfaces in ophys: `MockSegmentationInterface` [PR #1067](https://github.com/catalystneuro/neuroconv/pull/1067) -* Added automated EFS volume creation and mounting to the `submit_aws_job` helper function. [PR #1018](https://github.com/catalystneuro/neuroconv/pull/1018) ### Improvements From 3fb37880650348e878f310bee62e4f1875699779 Mon Sep 17 00:00:00 2001 From: Heberto Mayorquin Date: Sat, 14 Sep 2024 13:01:32 -0600 Subject: [PATCH 4/4] readd wrongly removed check on metadata --- src/neuroconv/tools/testing/data_interface_mixins.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/neuroconv/tools/testing/data_interface_mixins.py b/src/neuroconv/tools/testing/data_interface_mixins.py index 7763ba8eb..d3107650b 100644 --- a/src/neuroconv/tools/testing/data_interface_mixins.py +++ b/src/neuroconv/tools/testing/data_interface_mixins.py @@ -105,6 +105,7 @@ def test_no_metadata_mutation(self, setup_interface): metadata_before_add_method = deepcopy(metadata) self.interface.add_to_nwbfile(nwbfile=nwbfile, metadata=metadata, **self.conversion_options) + assert metadata == metadata_before_add_method def check_run_conversion_with_backend_configuration( self, nwbfile_path: str, backend: Literal["hdf5", "zarr"] = "hdf5"