From 9448f95f1b92b4035b792a7d87571dedcd2fb044 Mon Sep 17 00:00:00 2001 From: Heberto Mayorquin Date: Mon, 11 Nov 2024 10:02:38 -0600 Subject: [PATCH 1/6] Remove deprecations and add docstring to `BaseImagingExtractorInterface` (#1126) --- CHANGELOG.md | 1 + .../behavior/fictrac/fictracdatainterface.py | 14 ------ .../ecephys/baselfpextractorinterface.py | 4 -- .../baserecordingextractorinterface.py | 4 -- .../ophys/baseimagingextractorinterface.py | 45 ++++++++++--------- src/neuroconv/tools/neo/neo.py | 36 --------------- .../tools/spikeinterface/spikeinterface.py | 38 ---------------- .../tools/testing/mock_interfaces.py | 4 +- .../test_baseimagingextractorinterface.py | 15 ------- 9 files changed, 28 insertions(+), 133 deletions(-) delete mode 100644 tests/test_ophys/test_baseimagingextractorinterface.py diff --git a/CHANGELOG.md b/CHANGELOG.md index fa679434a..75c6ea917 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,7 @@ # Upcoming ## Deprecations +* Completely removed compression settings from most places[PR #1126](https://github.com/catalystneuro/neuroconv/pull/1126) ## Bug Fixes diff --git a/src/neuroconv/datainterfaces/behavior/fictrac/fictracdatainterface.py b/src/neuroconv/datainterfaces/behavior/fictrac/fictracdatainterface.py index 1b9686fd1..1d822f919 100644 --- a/src/neuroconv/datainterfaces/behavior/fictrac/fictracdatainterface.py +++ b/src/neuroconv/datainterfaces/behavior/fictrac/fictracdatainterface.py @@ -1,6 +1,5 @@ import json import re -import warnings from datetime import datetime, timezone from pathlib import Path from typing import Optional, Union @@ -210,8 +209,6 @@ def add_to_nwbfile( self, nwbfile: NWBFile, metadata: Optional[dict] = None, - compression: Optional[str] = None, # TODO: remove completely after 10/1/2024 - compression_opts: Optional[int] = None, # TODO: remove completely after 10/1/2024 ): """ Parameters @@ -223,17 +220,6 @@ def add_to_nwbfile( """ import pandas as pd - # TODO: remove completely after 10/1/2024 - if compression is not None or compression_opts is not None: - warnings.warn( - message=( - "Specifying compression methods and their options at the level of tool functions has been deprecated. " - "Please use the `configure_backend` tool function for this purpose." - ), - category=DeprecationWarning, - stacklevel=2, - ) - fictrac_data_df = pd.read_csv(self.file_path, sep=",", header=None, names=self.columns_in_dat_file) # Get the timestamps diff --git a/src/neuroconv/datainterfaces/ecephys/baselfpextractorinterface.py b/src/neuroconv/datainterfaces/ecephys/baselfpextractorinterface.py index 7ce6bb9e4..af16601bb 100644 --- a/src/neuroconv/datainterfaces/ecephys/baselfpextractorinterface.py +++ b/src/neuroconv/datainterfaces/ecephys/baselfpextractorinterface.py @@ -26,8 +26,6 @@ def add_to_nwbfile( starting_time: Optional[float] = None, write_as: Literal["raw", "lfp", "processed"] = "lfp", write_electrical_series: bool = True, - compression: Optional[str] = None, # TODO: remove completely after 10/1/2024 - compression_opts: Optional[int] = None, iterator_type: str = "v2", iterator_opts: Optional[dict] = None, ): @@ -38,8 +36,6 @@ def add_to_nwbfile( starting_time=starting_time, write_as=write_as, write_electrical_series=write_electrical_series, - compression=compression, - compression_opts=compression_opts, iterator_type=iterator_type, iterator_opts=iterator_opts, ) diff --git a/src/neuroconv/datainterfaces/ecephys/baserecordingextractorinterface.py b/src/neuroconv/datainterfaces/ecephys/baserecordingextractorinterface.py index e2c747378..6d0df14c1 100644 --- a/src/neuroconv/datainterfaces/ecephys/baserecordingextractorinterface.py +++ b/src/neuroconv/datainterfaces/ecephys/baserecordingextractorinterface.py @@ -308,8 +308,6 @@ def add_to_nwbfile( starting_time: Optional[float] = None, write_as: Literal["raw", "lfp", "processed"] = "raw", write_electrical_series: bool = True, - compression: Optional[str] = None, # TODO: remove completely after 10/1/2024 - compression_opts: Optional[int] = None, iterator_type: Optional[str] = "v2", iterator_opts: Optional[dict] = None, always_write_timestamps: bool = False, @@ -388,8 +386,6 @@ def add_to_nwbfile( write_as=write_as, write_electrical_series=write_electrical_series, es_key=self.es_key, - compression=compression, - compression_opts=compression_opts, iterator_type=iterator_type, iterator_opts=iterator_opts, always_write_timestamps=always_write_timestamps, diff --git a/src/neuroconv/datainterfaces/ophys/baseimagingextractorinterface.py b/src/neuroconv/datainterfaces/ophys/baseimagingextractorinterface.py index 5125af3cc..9f88b861f 100644 --- a/src/neuroconv/datainterfaces/ophys/baseimagingextractorinterface.py +++ b/src/neuroconv/datainterfaces/ophys/baseimagingextractorinterface.py @@ -1,6 +1,5 @@ """Author: Ben Dichter.""" -import warnings from typing import Literal, Optional import numpy as np @@ -46,17 +45,9 @@ def __init__( self.photon_series_type = photon_series_type def get_metadata_schema( - self, photon_series_type: Optional[Literal["OnePhotonSeries", "TwoPhotonSeries"]] = None + self, ) -> dict: - if photon_series_type is not None: - warnings.warn( - "The 'photon_series_type' argument is deprecated and will be removed in a future version. " - "Please set 'photon_series_type' during the initialization of the BaseImagingExtractorInterface instance.", - DeprecationWarning, - stacklevel=2, - ) - self.photon_series_type = photon_series_type metadata_schema = super().get_metadata_schema() metadata_schema["required"] = ["Ophys"] @@ -100,18 +91,9 @@ def get_metadata_schema( return metadata_schema def get_metadata( - self, photon_series_type: Optional[Literal["OnePhotonSeries", "TwoPhotonSeries"]] = None + self, ) -> DeepDict: - if photon_series_type is not None: - warnings.warn( - "The 'photon_series_type' argument is deprecated and will be removed in a future version. " - "Please set 'photon_series_type' during the initialization of the BaseImagingExtractorInterface instance.", - DeprecationWarning, - stacklevel=2, - ) - self.photon_series_type = photon_series_type - from ...tools.roiextractors import get_nwb_imaging_metadata metadata = super().get_metadata() @@ -147,6 +129,29 @@ def add_to_nwbfile( stub_test: bool = False, stub_frames: int = 100, ): + """ + Add imaging data to the NWB file + + Parameters + ---------- + nwbfile : NWBFile + The NWB file where the imaging data will be added. + metadata : dict, optional + Metadata for the NWBFile, by default None. + photon_series_type : {"TwoPhotonSeries", "OnePhotonSeries"}, optional + The type of photon series to be added, by default "TwoPhotonSeries". + photon_series_index : int, optional + The index of the photon series in the provided imaging data, by default 0. + parent_container : {"acquisition", "processing/ophys"}, optional + Specifies the parent container to which the photon series should be added, either as part of "acquisition" or + under the "processing/ophys" module, by default "acquisition". + stub_test : bool, optional + If True, only writes a small subset of frames for testing purposes, by default False. + stub_frames : int, optional + The number of frames to write when stub_test is True. Will use min(stub_frames, total_frames) to avoid + exceeding available frames, by default 100. + """ + from ...tools.roiextractors import add_imaging_to_nwbfile if stub_test: diff --git a/src/neuroconv/tools/neo/neo.py b/src/neuroconv/tools/neo/neo.py index 220c64de0..ccef706e5 100644 --- a/src/neuroconv/tools/neo/neo.py +++ b/src/neuroconv/tools/neo/neo.py @@ -214,7 +214,6 @@ def add_icephys_recordings( icephys_experiment_type: str = "voltage_clamp", stimulus_type: str = "not described", skip_electrodes: tuple[int] = (), - compression: Optional[str] = None, # TODO: remove completely after 10/1/2024 ): """ Add icephys recordings (stimulus/response pairs) to nwbfile object. @@ -230,16 +229,6 @@ def add_icephys_recordings( skip_electrodes : tuple, default: () Electrode IDs to skip. """ - # TODO: remove completely after 10/1/2024 - if compression is not None: - warn( - message=( - "Specifying compression methods and their options at the level of tool functions has been deprecated. " - "Please use the `configure_backend` tool function for this purpose." - ), - category=DeprecationWarning, - stacklevel=2, - ) n_segments = get_number_of_segments(neo_reader, block=0) @@ -380,7 +369,6 @@ def add_neo_to_nwb( neo_reader, nwbfile: pynwb.NWBFile, metadata: dict = None, - compression: Optional[str] = None, # TODO: remove completely after 10/1/2024 icephys_experiment_type: str = "voltage_clamp", stimulus_type: Optional[str] = None, skip_electrodes: tuple[int] = (), @@ -409,15 +397,6 @@ def add_neo_to_nwb( assert isinstance(nwbfile, pynwb.NWBFile), "'nwbfile' should be of type pynwb.NWBFile" # TODO: remove completely after 10/1/2024 - if compression is not None: - warn( - message=( - "Specifying compression methods and their options at the level of tool functions has been deprecated. " - "Please use the `configure_backend` tool function for this purpose." - ), - category=DeprecationWarning, - stacklevel=2, - ) add_device_from_metadata(nwbfile=nwbfile, modality="Icephys", metadata=metadata) @@ -443,7 +422,6 @@ def write_neo_to_nwb( overwrite: bool = False, nwbfile=None, metadata: dict = None, - compression: Optional[str] = None, # TODO: remove completely after 10/1/2024 icephys_experiment_type: Optional[str] = None, stimulus_type: Optional[str] = None, skip_electrodes: Optional[tuple] = (), @@ -499,9 +477,6 @@ def write_neo_to_nwb( Note that data intended to be added to the electrodes table of the NWBFile should be set as channel properties in the RecordingExtractor object. - compression: str (optional, defaults to "gzip") - Type of compression to use. Valid types are "gzip" and "lzf". - Set to None to disable all compression. icephys_experiment_type: str (optional) Type of Icephys experiment. Allowed types are: 'voltage_clamp', 'current_clamp' and 'izero'. If no value is passed, 'voltage_clamp' is used as default. @@ -518,17 +493,6 @@ def write_neo_to_nwb( assert save_path is None or nwbfile is None, "Either pass a save_path location, or nwbfile object, but not both!" - # TODO: remove completely after 10/1/2024 - if compression is not None: - warn( - message=( - "Specifying compression methods and their options at the level of tool functions has been deprecated. " - "Please use the `configure_backend` tool function for this purpose." - ), - category=DeprecationWarning, - stacklevel=2, - ) - if metadata is None: metadata = get_nwb_metadata(neo_reader=neo_reader) diff --git a/src/neuroconv/tools/spikeinterface/spikeinterface.py b/src/neuroconv/tools/spikeinterface/spikeinterface.py index 1be86862a..5aa3c8925 100644 --- a/src/neuroconv/tools/spikeinterface/spikeinterface.py +++ b/src/neuroconv/tools/spikeinterface/spikeinterface.py @@ -749,8 +749,6 @@ def add_electrical_series( write_as: Literal["raw", "processed", "lfp"] = "raw", es_key: str = None, write_scaled: bool = False, - compression: Optional[str] = None, - compression_opts: Optional[int] = None, iterator_type: Optional[str] = "v2", iterator_opts: Optional[dict] = None, ): @@ -772,8 +770,6 @@ def add_electrical_series( write_as=write_as, es_key=es_key, write_scaled=write_scaled, - compression=compression, - compression_opts=compression_opts, iterator_type=iterator_type, iterator_opts=iterator_opts, ) @@ -810,8 +806,6 @@ def add_electrical_series_to_nwbfile( write_as: Literal["raw", "processed", "lfp"] = "raw", es_key: str = None, write_scaled: bool = False, - compression: Optional[str] = None, - compression_opts: Optional[int] = None, iterator_type: Optional[str] = "v2", iterator_opts: Optional[dict] = None, always_write_timestamps: bool = False, @@ -847,7 +841,6 @@ def add_electrical_series_to_nwbfile( write_scaled : bool, default: False If True, writes the traces in uV with the right conversion. If False , the data is stored as it is and the right conversions factors are added to the nwbfile. - Only applies to compression="gzip". Controls the level of the GZIP. iterator_type: {"v2", None}, default: 'v2' The type of DataChunkIterator to use. 'v1' is the original DataChunkIterator of the hdmf data_utils. @@ -868,16 +861,6 @@ def add_electrical_series_to_nwbfile( Missing keys in an element of metadata['Ecephys']['ElectrodeGroup'] will be auto-populated with defaults whenever possible. """ - # TODO: remove completely after 10/1/2024 - if compression is not None or compression_opts is not None: - warnings.warn( - message=( - "Specifying compression methods and their options at the level of tool functions has been deprecated. " - "Please use the `configure_backend` tool function for this purpose." - ), - category=DeprecationWarning, - stacklevel=2, - ) assert write_as in [ "raw", @@ -1042,8 +1025,6 @@ def add_recording( es_key: Optional[str] = None, write_electrical_series: bool = True, write_scaled: bool = False, - compression: Optional[str] = "gzip", - compression_opts: Optional[int] = None, iterator_type: str = "v2", iterator_opts: Optional[dict] = None, ): @@ -1065,8 +1046,6 @@ def add_recording( es_key=es_key, write_electrical_series=write_electrical_series, write_scaled=write_scaled, - compression=compression, - compression_opts=compression_opts, iterator_type=iterator_type, iterator_opts=iterator_opts, ) @@ -1081,8 +1060,6 @@ def add_recording_to_nwbfile( es_key: Optional[str] = None, write_electrical_series: bool = True, write_scaled: bool = False, - compression: Optional[str] = "gzip", - compression_opts: Optional[int] = None, iterator_type: str = "v2", iterator_opts: Optional[dict] = None, always_write_timestamps: bool = False, @@ -1163,8 +1140,6 @@ def add_recording_to_nwbfile( write_as=write_as, es_key=es_key, write_scaled=write_scaled, - compression=compression, - compression_opts=compression_opts, iterator_type=iterator_type, iterator_opts=iterator_opts, always_write_timestamps=always_write_timestamps, @@ -1183,8 +1158,6 @@ def write_recording( es_key: Optional[str] = None, write_electrical_series: bool = True, write_scaled: bool = False, - compression: Optional[str] = "gzip", - compression_opts: Optional[int] = None, iterator_type: Optional[str] = "v2", iterator_opts: Optional[dict] = None, ): @@ -1209,8 +1182,6 @@ def write_recording( es_key=es_key, write_electrical_series=write_electrical_series, write_scaled=write_scaled, - compression=compression, - compression_opts=compression_opts, iterator_type=iterator_type, iterator_opts=iterator_opts, ) @@ -1228,8 +1199,6 @@ def write_recording_to_nwbfile( es_key: Optional[str] = None, write_electrical_series: bool = True, write_scaled: bool = False, - compression: Optional[str] = "gzip", - compression_opts: Optional[int] = None, iterator_type: Optional[str] = "v2", iterator_opts: Optional[dict] = None, ) -> pynwb.NWBFile: @@ -1303,11 +1272,6 @@ def write_recording_to_nwbfile( and electrodes are written to NWB. write_scaled: bool, default: True If True, writes the scaled traces (return_scaled=True) - compression: {None, 'gzip', 'lzp'}, default: 'gzip' - Type of compression to use. Set to None to disable all compression. - To use the `configure_backend` function, you should set this to None. - compression_opts: int, optional, default: 4 - Only applies to compression="gzip". Controls the level of the GZIP. iterator_type: {"v2", "v1", None} The type of DataChunkIterator to use. 'v1' is the original DataChunkIterator of the hdmf data_utils. @@ -1348,8 +1312,6 @@ def write_recording_to_nwbfile( es_key=es_key, write_electrical_series=write_electrical_series, write_scaled=write_scaled, - compression=compression, - compression_opts=compression_opts, iterator_type=iterator_type, iterator_opts=iterator_opts, ) diff --git a/src/neuroconv/tools/testing/mock_interfaces.py b/src/neuroconv/tools/testing/mock_interfaces.py index 4ba7bb639..44d1adf61 100644 --- a/src/neuroconv/tools/testing/mock_interfaces.py +++ b/src/neuroconv/tools/testing/mock_interfaces.py @@ -265,9 +265,9 @@ def __init__( self.verbose = verbose self.photon_series_type = photon_series_type - def get_metadata(self, photon_series_type: Optional[Literal["OnePhotonSeries", "TwoPhotonSeries"]] = None) -> dict: + def get_metadata(self) -> dict: session_start_time = datetime.now().astimezone() - metadata = super().get_metadata(photon_series_type=photon_series_type) + metadata = super().get_metadata() metadata["NWBFile"]["session_start_time"] = session_start_time return metadata diff --git a/tests/test_ophys/test_baseimagingextractorinterface.py b/tests/test_ophys/test_baseimagingextractorinterface.py deleted file mode 100644 index 863a978d2..000000000 --- a/tests/test_ophys/test_baseimagingextractorinterface.py +++ /dev/null @@ -1,15 +0,0 @@ -from hdmf.testing import TestCase - -from neuroconv.tools.testing.mock_interfaces import MockImagingInterface - - -class TestBaseImagingExtractorInterface(TestCase): - def setUp(self): - self.mock_imaging_interface = MockImagingInterface() - - def test_photon_series_type_warning_triggered_in_get_metadata(self): - with self.assertWarnsWith( - warn_type=DeprecationWarning, - exc_msg="The 'photon_series_type' argument is deprecated and will be removed in a future version. Please set 'photon_series_type' during the initialization of the BaseImagingExtractorInterface instance.", - ): - self.mock_imaging_interface.get_metadata(photon_series_type="TwoPhotonSeries") From 6960872de0b11f9f04d4f0efecbec4aa9c010c12 Mon Sep 17 00:00:00 2001 From: Heberto Mayorquin Date: Mon, 11 Nov 2024 14:17:24 -0600 Subject: [PATCH 2/6] Add `always_write_timestamps` conversion option to imaging interfaces (#1125) --- CHANGELOG.md | 3 +- pyproject.toml | 26 +++++++------- .../behavior/video/videodatainterface.py | 13 ------- .../ophys/baseimagingextractorinterface.py | 2 ++ .../tools/roiextractors/roiextractors.py | 34 +++++++++++++++---- .../tools/testing/mock_interfaces.py | 2 ++ tests/test_ophys/test_ophys_interfaces.py | 14 +++++++- 7 files changed, 59 insertions(+), 35 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 75c6ea917..a06ddf300 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ ## Bug Fixes ## Features +* Imaging interfaces have a new conversion option `always_write_timestamps` that can be used to force writing timestamps even if neuroconv's heuristics indicates regular sampling rate [PR #1125](https://github.com/catalystneuro/neuroconv/pull/1125) ## Improvements @@ -46,7 +47,7 @@ * 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) * Added a `MockSortingInterface` for testing purposes. [PR #1065](https://github.com/catalystneuro/neuroconv/pull/1065) -* BaseRecordingInterfaces have a new conversion options `always_write_timestamps` that ca be used to force writing timestamps even if neuroconv heuristic indicates regular sampling rate [PR #1091](https://github.com/catalystneuro/neuroconv/pull/1091) +* BaseRecordingInterfaces have a new conversion options `always_write_timestamps` that can be used to force writing timestamps even if neuroconv heuristic indicates regular sampling rate [PR #1091](https://github.com/catalystneuro/neuroconv/pull/1091) ## Improvements diff --git a/pyproject.toml b/pyproject.toml index a83380467..5efd432f5 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -270,50 +270,50 @@ icephys = [ ## Ophys brukertiff = [ - "roiextractors>=0.5.7", + "roiextractors>=0.5.10", "tifffile>=2023.3.21", ] caiman = [ - "roiextractors>=0.5.7", + "roiextractors>=0.5.10", ] cnmfe = [ - "roiextractors>=0.5.7", + "roiextractors>=0.5.10", ] extract = [ - "roiextractors>=0.5.7", + "roiextractors>=0.5.10", ] hdf5 = [ - "roiextractors>=0.5.7", + "roiextractors>=0.5.10", ] micromanagertiff = [ - "roiextractors>=0.5.7", + "roiextractors>=0.5.10", "tifffile>=2023.3.21", ] miniscope = [ "natsort>=8.3.1", "ndx-miniscope>=0.5.1", - "roiextractors>=0.5.7", + "roiextractors>=0.5.10", ] sbx = [ - "roiextractors>=0.5.7", + "roiextractors>=0.5.10", ] scanimage = [ - "roiextractors>=0.5.7", + "roiextractors>=0.5.10", "scanimage-tiff-reader>=1.4.1", ] sima = [ - "roiextractors>=0.5.7", + "roiextractors>=0.5.10", ] suite2p = [ - "roiextractors>=0.5.7", + "roiextractors>=0.5.10", ] tdt_fp = [ "ndx-fiber-photometry", - "roiextractors>=0.5.7", + "roiextractors>=0.5.10", "tdt", ] tiff = [ - "roiextractors>=0.5.7", + "roiextractors>=0.5.9", "tiffile>=2018.10.18", ] ophys = [ diff --git a/src/neuroconv/datainterfaces/behavior/video/videodatainterface.py b/src/neuroconv/datainterfaces/behavior/video/videodatainterface.py index a544f9c27..aaa875f3e 100644 --- a/src/neuroconv/datainterfaces/behavior/video/videodatainterface.py +++ b/src/neuroconv/datainterfaces/behavior/video/videodatainterface.py @@ -269,8 +269,6 @@ def add_to_nwbfile( chunk_data: bool = True, module_name: Optional[str] = None, module_description: Optional[str] = None, - compression: Optional[str] = "gzip", - compression_options: Optional[int] = None, ): """ Convert the video data files to :py:class:`~pynwb.image.ImageSeries` and write them in the @@ -431,17 +429,6 @@ def add_to_nwbfile( pbar.update(1) iterable = video - # TODO: remove completely after 03/1/2024 - if compression is not None or compression_options is not None: - warnings.warn( - message=( - "Specifying compression methods and their options for this interface has been deprecated. " - "Please use the `configure_backend` tool function for this purpose." - ), - category=DeprecationWarning, - stacklevel=2, - ) - image_series_kwargs.update(data=iterable) if timing_type == "starting_time and rate": diff --git a/src/neuroconv/datainterfaces/ophys/baseimagingextractorinterface.py b/src/neuroconv/datainterfaces/ophys/baseimagingextractorinterface.py index 9f88b861f..0019b8bd7 100644 --- a/src/neuroconv/datainterfaces/ophys/baseimagingextractorinterface.py +++ b/src/neuroconv/datainterfaces/ophys/baseimagingextractorinterface.py @@ -128,6 +128,7 @@ def add_to_nwbfile( parent_container: Literal["acquisition", "processing/ophys"] = "acquisition", stub_test: bool = False, stub_frames: int = 100, + always_write_timestamps: bool = False, ): """ Add imaging data to the NWB file @@ -167,4 +168,5 @@ def add_to_nwbfile( photon_series_type=photon_series_type, photon_series_index=photon_series_index, parent_container=parent_container, + always_write_timestamps=always_write_timestamps, ) diff --git a/src/neuroconv/tools/roiextractors/roiextractors.py b/src/neuroconv/tools/roiextractors/roiextractors.py index f28631c77..27d3b5f9c 100644 --- a/src/neuroconv/tools/roiextractors/roiextractors.py +++ b/src/neuroconv/tools/roiextractors/roiextractors.py @@ -445,6 +445,7 @@ def add_photon_series_to_nwbfile( parent_container: Literal["acquisition", "processing/ophys"] = "acquisition", iterator_type: Optional[str] = "v2", iterator_options: Optional[dict] = None, + always_write_timestamps: bool = False, ) -> NWBFile: """ Auxiliary static method for nwbextractor. @@ -472,6 +473,11 @@ def add_photon_series_to_nwbfile( iterator_type: str, default: 'v2' The type of iterator to use when adding the photon series to the NWB file. iterator_options: dict, optional + always_write_timestamps : bool, default: False + Set to True to always write timestamps. + By default (False), the function checks if the timestamps are uniformly sampled, and if so, stores the data + using a regular sampling rate instead of explicit timestamps. If set to True, timestamps will be written + explicitly, regardless of whether the sampling rate is uniform. Returns ------- @@ -530,16 +536,23 @@ def add_photon_series_to_nwbfile( photon_series_kwargs.update(dimension=imaging.get_image_size()) # Add timestamps or rate - if imaging.has_time_vector(): + if always_write_timestamps: timestamps = imaging.frame_to_time(np.arange(imaging.get_num_frames())) - estimated_rate = calculate_regular_series_rate(series=timestamps) + photon_series_kwargs.update(timestamps=timestamps) + else: + imaging_has_timestamps = imaging.has_time_vector() + if imaging_has_timestamps: + timestamps = imaging.frame_to_time(np.arange(imaging.get_num_frames())) + estimated_rate = calculate_regular_series_rate(series=timestamps) + starting_time = timestamps[0] + else: + estimated_rate = float(imaging.get_sampling_frequency()) + starting_time = 0.0 + if estimated_rate: - photon_series_kwargs.update(starting_time=timestamps[0], rate=estimated_rate) + photon_series_kwargs.update(rate=estimated_rate, starting_time=starting_time) else: - photon_series_kwargs.update(timestamps=timestamps, rate=None) - else: - rate = float(imaging.get_sampling_frequency()) - photon_series_kwargs.update(rate=rate) + photon_series_kwargs.update(timestamps=timestamps) # Add the photon series to the nwbfile (either as OnePhotonSeries or TwoPhotonSeries) photon_series = dict( @@ -682,6 +695,7 @@ def add_imaging_to_nwbfile( iterator_type: Optional[str] = "v2", iterator_options: Optional[dict] = None, parent_container: Literal["acquisition", "processing/ophys"] = "acquisition", + always_write_timestamps: bool = False, ) -> NWBFile: """ Add imaging data from an ImagingExtractor object to an NWBFile. @@ -705,6 +719,11 @@ def add_imaging_to_nwbfile( parent_container : {"acquisition", "processing/ophys"}, optional Specifies the parent container to which the photon series should be added, either as part of "acquisition" or under the "processing/ophys" module, by default "acquisition". + always_write_timestamps : bool, default: False + Set to True to always write timestamps. + By default (False), the function checks if the timestamps are uniformly sampled, and if so, stores the data + using a regular sampling rate instead of explicit timestamps. If set to True, timestamps will be written + explicitly, regardless of whether the sampling rate is uniform. Returns ------- @@ -722,6 +741,7 @@ def add_imaging_to_nwbfile( iterator_type=iterator_type, iterator_options=iterator_options, parent_container=parent_container, + always_write_timestamps=always_write_timestamps, ) return nwbfile diff --git a/src/neuroconv/tools/testing/mock_interfaces.py b/src/neuroconv/tools/testing/mock_interfaces.py index 44d1adf61..dd3ec12c2 100644 --- a/src/neuroconv/tools/testing/mock_interfaces.py +++ b/src/neuroconv/tools/testing/mock_interfaces.py @@ -260,6 +260,7 @@ def __init__( sampling_frequency=sampling_frequency, dtype=dtype, verbose=verbose, + seed=seed, ) self.verbose = verbose @@ -334,6 +335,7 @@ def __init__( has_deconvolved_signal=has_deconvolved_signal, has_neuropil_signal=has_neuropil_signal, verbose=verbose, + seed=seed, ) def get_metadata(self) -> dict: diff --git a/tests/test_ophys/test_ophys_interfaces.py b/tests/test_ophys/test_ophys_interfaces.py index 4381faf8b..3ea329e5f 100644 --- a/tests/test_ophys/test_ophys_interfaces.py +++ b/tests/test_ophys/test_ophys_interfaces.py @@ -1,3 +1,5 @@ +import numpy as np + from neuroconv.tools.testing.data_interface_mixins import ( ImagingExtractorInterfaceTestMixin, SegmentationExtractorInterfaceTestMixin, @@ -12,7 +14,17 @@ class TestMockImagingInterface(ImagingExtractorInterfaceTestMixin): data_interface_cls = MockImagingInterface interface_kwargs = dict() - # TODO: fix this by setting a seed on the dummy imaging extractor + def test_always_write_timestamps(self, setup_interface): + # By default the MockImagingInterface has a uniform sampling rate + + nwbfile = self.interface.create_nwbfile(always_write_timestamps=True) + two_photon_series = nwbfile.acquisition["TwoPhotonSeries"] + imaging = self.interface.imaging_extractor + expected_timestamps = imaging.frame_to_time(np.arange(imaging.get_num_frames())) + + np.testing.assert_array_equal(two_photon_series.timestamps[:], expected_timestamps) + + # Remove this after roiextractors 0.5.10 is released def test_all_conversion_checks(self): pass From e3cde1f38d9de4f970ffdb1ffd2f73078bb04e0b Mon Sep 17 00:00:00 2001 From: Heberto Mayorquin Date: Thu, 14 Nov 2024 07:47:34 -0600 Subject: [PATCH 3/6] Support datetime in conversion options (#1139) --- CHANGELOG.md | 1 + src/neuroconv/basedatainterface.py | 4 ++- src/neuroconv/nwbconverter.py | 29 ++++++++++++------ .../tools/testing/mock_interfaces.py | 21 +++++++++++++ src/neuroconv/utils/json_schema.py | 17 +++++++++++ .../test_minimal/test_interface_validation.py | 30 +++++++++++++++++++ 6 files changed, 92 insertions(+), 10 deletions(-) create mode 100644 tests/test_minimal/test_interface_validation.py diff --git a/CHANGELOG.md b/CHANGELOG.md index a06ddf300..cdc70223f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ * Completely removed compression settings from most places[PR #1126](https://github.com/catalystneuro/neuroconv/pull/1126) ## Bug Fixes +* datetime objects now can be validated as conversion options [#1139](https://github.com/catalystneuro/neuroconv/pull/1126) ## Features * Imaging interfaces have a new conversion option `always_write_timestamps` that can be used to force writing timestamps even if neuroconv's heuristics indicates regular sampling rate [PR #1125](https://github.com/catalystneuro/neuroconv/pull/1125) diff --git a/src/neuroconv/basedatainterface.py b/src/neuroconv/basedatainterface.py index adcec89b5..272abbd0c 100644 --- a/src/neuroconv/basedatainterface.py +++ b/src/neuroconv/basedatainterface.py @@ -126,7 +126,7 @@ def create_nwbfile(self, metadata: Optional[dict] = None, **conversion_options) return nwbfile @abstractmethod - def add_to_nwbfile(self, nwbfile: NWBFile, **conversion_options) -> None: + def add_to_nwbfile(self, nwbfile: NWBFile, metadata: Optional[dict], **conversion_options) -> None: """ Define a protocol for mapping the data from this interface to NWB neurodata objects. @@ -136,6 +136,8 @@ def add_to_nwbfile(self, nwbfile: NWBFile, **conversion_options) -> None: ---------- nwbfile : pynwb.NWBFile The in-memory object to add the data to. + metadata : dict + Metadata dictionary with information used to create the NWBFile. **conversion_options Additional keyword arguments to pass to the `.add_to_nwbfile` method. """ diff --git a/src/neuroconv/nwbconverter.py b/src/neuroconv/nwbconverter.py index 1f3e7c9f8..fe1b09915 100644 --- a/src/neuroconv/nwbconverter.py +++ b/src/neuroconv/nwbconverter.py @@ -29,7 +29,11 @@ unroot_schema, ) from .utils.dict import DeepDict -from .utils.json_schema import _NWBMetaDataEncoder, _NWBSourceDataEncoder +from .utils.json_schema import ( + _NWBConversionOptionsEncoder, + _NWBMetaDataEncoder, + _NWBSourceDataEncoder, +) class NWBConverter: @@ -63,11 +67,10 @@ def validate_source(cls, source_data: dict[str, dict], verbose: bool = True): def _validate_source_data(self, source_data: dict[str, dict], verbose: bool = True): + # We do this to ensure that python objects are in string format for the JSON schema encoder = _NWBSourceDataEncoder() - # The encoder produces a serialized object, so we deserialized it for comparison - - serialized_source_data = encoder.encode(source_data) - decoded_source_data = json.loads(serialized_source_data) + encoded_source_data = encoder.encode(source_data) + decoded_source_data = json.loads(encoded_source_data) validate(instance=decoded_source_data, schema=self.get_source_schema()) if verbose: @@ -106,9 +109,10 @@ def get_metadata(self) -> DeepDict: def validate_metadata(self, metadata: dict[str, dict], append_mode: bool = False): """Validate metadata against Converter metadata_schema.""" encoder = _NWBMetaDataEncoder() - # The encoder produces a serialized object, so we deserialized it for comparison - serialized_metadata = encoder.encode(metadata) - decoded_metadata = json.loads(serialized_metadata) + + # We do this to ensure that python objects are in string format for the JSON schema + encoded_metadta = encoder.encode(metadata) + decoded_metadata = json.loads(encoded_metadta) metadata_schema = self.get_metadata_schema() if append_mode: @@ -138,7 +142,14 @@ def get_conversion_options_schema(self) -> dict: def validate_conversion_options(self, conversion_options: dict[str, dict]): """Validate conversion_options against Converter conversion_options_schema.""" - validate(instance=conversion_options or {}, schema=self.get_conversion_options_schema()) + + conversion_options = conversion_options or dict() + + # We do this to ensure that python objects are in string format for the JSON schema + encoded_conversion_options = _NWBConversionOptionsEncoder().encode(conversion_options) + decoded_conversion_options = json.loads(encoded_conversion_options) + + validate(instance=decoded_conversion_options, schema=self.get_conversion_options_schema()) if self.verbose: print("conversion_options is valid!") diff --git a/src/neuroconv/tools/testing/mock_interfaces.py b/src/neuroconv/tools/testing/mock_interfaces.py index dd3ec12c2..0652284e7 100644 --- a/src/neuroconv/tools/testing/mock_interfaces.py +++ b/src/neuroconv/tools/testing/mock_interfaces.py @@ -6,6 +6,7 @@ from pynwb.base import DynamicTable from .mock_ttl_signals import generate_mock_ttl_signal +from ...basedatainterface import BaseDataInterface from ...basetemporalalignmentinterface import BaseTemporalAlignmentInterface from ...datainterfaces import SpikeGLXNIDQInterface from ...datainterfaces.ecephys.baserecordingextractorinterface import ( @@ -23,6 +24,26 @@ from ...utils import ArrayType, get_json_schema_from_method_signature +class MockInterface(BaseDataInterface): + """ + A mock interface for testing basic command passing without side effects. + """ + + def __init__(self, verbose: bool = False, **source_data): + + super().__init__(verbose=verbose, **source_data) + + def get_metadata(self) -> dict: + metadata = super().get_metadata() + session_start_time = datetime.now().astimezone() + metadata["NWBFile"]["session_start_time"] = session_start_time + return metadata + + def add_to_nwbfile(self, nwbfile: NWBFile, metadata: Optional[dict], **conversion_options): + + return None + + class MockBehaviorEventInterface(BaseTemporalAlignmentInterface): """ A mock behavior event interface for testing purposes. diff --git a/src/neuroconv/utils/json_schema.py b/src/neuroconv/utils/json_schema.py index 182558b98..07dc3321f 100644 --- a/src/neuroconv/utils/json_schema.py +++ b/src/neuroconv/utils/json_schema.py @@ -60,6 +60,23 @@ def default(self, obj): return super().default(obj) +class _NWBConversionOptionsEncoder(_NWBMetaDataEncoder): + """ + Custom JSON encoder for conversion options of the data interfaces and converters (i.e. kwargs). + + This encoder extends the default JSONEncoder class and provides custom serialization + for certain data types commonly used in interface source data. + """ + + def default(self, obj): + + # Over-write behaviors for Paths + if isinstance(obj, Path): + return str(obj) + + return super().default(obj) + + def get_base_schema( tag: Optional[str] = None, root: bool = False, diff --git a/tests/test_minimal/test_interface_validation.py b/tests/test_minimal/test_interface_validation.py new file mode 100644 index 000000000..1bc409b06 --- /dev/null +++ b/tests/test_minimal/test_interface_validation.py @@ -0,0 +1,30 @@ +from datetime import datetime +from typing import Optional + +from pynwb import NWBFile + +from neuroconv import ConverterPipe +from neuroconv.tools.testing.mock_interfaces import ( + MockInterface, +) + + +def test_conversion_options_validation(tmp_path): + + class InterfaceWithDateTimeConversionOptions(MockInterface): + "class for testing how a file with datetime object is validated" + + def add_to_nwbfile(self, nwbfile: NWBFile, metadata: Optional[dict], datetime_option: datetime): + pass + + interface = InterfaceWithDateTimeConversionOptions() + + nwbfile_path = tmp_path / "interface_test.nwb" + interface.run_conversion(nwbfile_path=nwbfile_path, datetime_option=datetime.now(), overwrite=True) + + data_interfaces = {"InterfaceWithDateTimeConversionOptions": interface} + conversion_options = {"InterfaceWithDateTimeConversionOptions": {"datetime_option": datetime.now()}} + converter = ConverterPipe(data_interfaces=data_interfaces) + + nwbfile_path = tmp_path / "converter_test.nwb" + converter.run_conversion(nwbfile_path=nwbfile_path, overwrite=True, conversion_options=conversion_options) From 56673dddd246f806ec2d7ee1911d86fcd21414ae Mon Sep 17 00:00:00 2001 From: Paul Adkisson Date: Fri, 15 Nov 2024 06:52:44 +1100 Subject: [PATCH 4/6] Added CSV support to DeepLabCutInterface (#1140) --- CHANGELOG.md | 1 + .../behavior/deeplabcut.rst | 5 +- .../behavior/deeplabcut/_dlc_utils.py | 41 ++++-------- .../deeplabcut/deeplabcutdatainterface.py | 30 +++++---- .../tools/testing/data_interface_mixins.py | 24 ------- .../behavior/test_behavior_interfaces.py | 65 ++++++++++++++++--- 6 files changed, 91 insertions(+), 75 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cdc70223f..0545001d1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ ## Features * Imaging interfaces have a new conversion option `always_write_timestamps` that can be used to force writing timestamps even if neuroconv's heuristics indicates regular sampling rate [PR #1125](https://github.com/catalystneuro/neuroconv/pull/1125) +* Added .csv support to DeepLabCutInterface [PR #1140](https://github.com/catalystneuro/neuroconv/pull/1140) ## Improvements diff --git a/docs/conversion_examples_gallery/behavior/deeplabcut.rst b/docs/conversion_examples_gallery/behavior/deeplabcut.rst index c20dd057d..64201ea72 100644 --- a/docs/conversion_examples_gallery/behavior/deeplabcut.rst +++ b/docs/conversion_examples_gallery/behavior/deeplabcut.rst @@ -8,6 +8,7 @@ Install NeuroConv with the additional dependencies necessary for reading DeepLab pip install "neuroconv[deeplabcut]" Convert DeepLabCut pose estimation data to NWB using :py:class:`~neuroconv.datainterfaces.behavior.deeplabcut.deeplabcutdatainterface.DeepLabCutInterface`. +This interface supports both .h5 and .csv output files from DeepLabCut. .. code-block:: python @@ -16,8 +17,8 @@ Convert DeepLabCut pose estimation data to NWB using :py:class:`~neuroconv.datai >>> from pathlib import Path >>> from neuroconv.datainterfaces import DeepLabCutInterface - >>> file_path = BEHAVIOR_DATA_PATH / "DLC" / "m3v1mp4DLC_resnet50_openfieldAug20shuffle1_30000.h5" - >>> config_file_path = BEHAVIOR_DATA_PATH / "DLC" / "config.yaml" + >>> file_path = BEHAVIOR_DATA_PATH / "DLC" / "open_field_without_video" / "m3v1mp4DLC_resnet50_openfieldAug20shuffle1_30000.h5" + >>> config_file_path = BEHAVIOR_DATA_PATH / "DLC" / "open_field_without_video" / "config.yaml" >>> interface = DeepLabCutInterface(file_path=file_path, config_file_path=config_file_path, subject_name="ind1", verbose=False) diff --git a/src/neuroconv/datainterfaces/behavior/deeplabcut/_dlc_utils.py b/src/neuroconv/datainterfaces/behavior/deeplabcut/_dlc_utils.py index 9e368fb39..5d1224e85 100644 --- a/src/neuroconv/datainterfaces/behavior/deeplabcut/_dlc_utils.py +++ b/src/neuroconv/datainterfaces/behavior/deeplabcut/_dlc_utils.py @@ -251,21 +251,6 @@ def _get_video_info_from_config_file(config_file_path: Path, vidname: str): return video_file_path, image_shape -def _get_pes_args( - *, - h5file: Path, - individual_name: str, -): - h5file = Path(h5file) - - _, scorer = h5file.stem.split("DLC") - scorer = "DLC" + scorer - - df = _ensure_individuals_in_header(pd.read_hdf(h5file), individual_name) - - return scorer, df - - def _write_pes_to_nwbfile( nwbfile, animal, @@ -339,23 +324,23 @@ def _write_pes_to_nwbfile( return nwbfile -def add_subject_to_nwbfile( +def _add_subject_to_nwbfile( nwbfile: NWBFile, - h5file: FilePath, + file_path: FilePath, individual_name: str, config_file: Optional[FilePath] = None, timestamps: Optional[Union[list, np.ndarray]] = None, pose_estimation_container_kwargs: Optional[dict] = None, ) -> NWBFile: """ - Given the subject name, add the DLC .h5 file to an in-memory NWBFile object. + Given the subject name, add the DLC output file (.h5 or .csv) to an in-memory NWBFile object. Parameters ---------- nwbfile : pynwb.NWBFile The in-memory nwbfile object to which the subject specific pose estimation series will be added. - h5file : str or path - Path to the DeepLabCut .h5 output file. + file_path : str or path + Path to the DeepLabCut .h5 or .csv output file. individual_name : str Name of the subject (whose pose is predicted) for single-animal DLC project. For multi-animal projects, the names from the DLC project will be used directly. @@ -371,18 +356,18 @@ def add_subject_to_nwbfile( nwbfile : pynwb.NWBFile nwbfile with pes written in the behavior module """ - h5file = Path(h5file) - - if "DLC" not in h5file.name or not h5file.suffix == ".h5": - raise IOError("The file passed in is not a DeepLabCut h5 data file.") + file_path = Path(file_path) - video_name, scorer = h5file.stem.split("DLC") + video_name, scorer = file_path.stem.split("DLC") scorer = "DLC" + scorer # TODO probably could be read directly with h5py # This requires pytables - data_frame_from_hdf5 = pd.read_hdf(h5file) - df = _ensure_individuals_in_header(data_frame_from_hdf5, individual_name) + if ".h5" in file_path.suffixes: + df = pd.read_hdf(file_path) + elif ".csv" in file_path.suffixes: + df = pd.read_csv(file_path, header=[0, 1, 2], index_col=0) + df = _ensure_individuals_in_header(df, individual_name) # Note the video here is a tuple of the video path and the image shape if config_file is not None: @@ -404,7 +389,7 @@ def add_subject_to_nwbfile( # Fetch the corresponding metadata pickle file, we extract the edges graph from here # TODO: This is the original implementation way to extract the file name but looks very brittle. Improve it - filename = str(h5file.parent / h5file.stem) + filename = str(file_path.parent / file_path.stem) for i, c in enumerate(filename[::-1]): if c.isnumeric(): break diff --git a/src/neuroconv/datainterfaces/behavior/deeplabcut/deeplabcutdatainterface.py b/src/neuroconv/datainterfaces/behavior/deeplabcut/deeplabcutdatainterface.py index 21b054e85..f45913061 100644 --- a/src/neuroconv/datainterfaces/behavior/deeplabcut/deeplabcutdatainterface.py +++ b/src/neuroconv/datainterfaces/behavior/deeplabcut/deeplabcutdatainterface.py @@ -5,6 +5,7 @@ from pydantic import FilePath, validate_call from pynwb.file import NWBFile +# import ndx_pose from ....basetemporalalignmentinterface import BaseTemporalAlignmentInterface @@ -13,7 +14,7 @@ class DeepLabCutInterface(BaseTemporalAlignmentInterface): display_name = "DeepLabCut" keywords = ("DLC",) - associated_suffixes = (".h5",) + associated_suffixes = (".h5", ".csv") info = "Interface for handling data from DeepLabCut." _timestamps = None @@ -21,8 +22,8 @@ class DeepLabCutInterface(BaseTemporalAlignmentInterface): @classmethod def get_source_schema(cls) -> dict: source_schema = super().get_source_schema() - source_schema["properties"]["file_path"]["description"] = "Path to the .h5 file output by dlc." - source_schema["properties"]["config_file_path"]["description"] = "Path to .yml config file" + source_schema["properties"]["file_path"]["description"] = "Path to the file output by dlc (.h5 or .csv)." + source_schema["properties"]["config_file_path"]["description"] = "Path to .yml config file." return source_schema @validate_call @@ -34,24 +35,25 @@ def __init__( verbose: bool = True, ): """ - Interface for writing DLC's h5 files to nwb using dlc2nwb. + Interface for writing DLC's output files to nwb using dlc2nwb. Parameters ---------- file_path : FilePath - path to the h5 file output by dlc. + Path to the file output by dlc (.h5 or .csv). config_file_path : FilePath, optional - path to .yml config file + Path to .yml config file subject_name : str, default: "ind1" - the name of the subject for which the :py:class:`~pynwb.file.NWBFile` is to be created. + The name of the subject for which the :py:class:`~pynwb.file.NWBFile` is to be created. verbose: bool, default: True - controls verbosity. + Controls verbosity. """ from ._dlc_utils import _read_config file_path = Path(file_path) - if "DLC" not in file_path.stem or ".h5" not in file_path.suffixes: - raise IOError("The file passed in is not a DeepLabCut h5 data file.") + suffix_is_valid = ".h5" in file_path.suffixes or ".csv" in file_path.suffixes + if not "DLC" in file_path.stem or not suffix_is_valid: + raise IOError("The file passed in is not a valid DeepLabCut output data file.") self.config_dict = dict() if config_file_path is not None: @@ -108,12 +110,14 @@ def add_to_nwbfile( nwb file to which the recording information is to be added metadata: dict metadata info for constructing the nwb file (optional). + container_name: str, default: "PoseEstimation" + Name of the container to store the pose estimation. """ - from ._dlc_utils import add_subject_to_nwbfile + from ._dlc_utils import _add_subject_to_nwbfile - add_subject_to_nwbfile( + _add_subject_to_nwbfile( nwbfile=nwbfile, - h5file=str(self.source_data["file_path"]), + file_path=str(self.source_data["file_path"]), individual_name=self.subject_name, config_file=self.source_data["config_file_path"], timestamps=self._timestamps, diff --git a/src/neuroconv/tools/testing/data_interface_mixins.py b/src/neuroconv/tools/testing/data_interface_mixins.py index 946b3fd6c..5187ff2e4 100644 --- a/src/neuroconv/tools/testing/data_interface_mixins.py +++ b/src/neuroconv/tools/testing/data_interface_mixins.py @@ -743,30 +743,6 @@ def test_interface_alignment(self): pass -class DeepLabCutInterfaceMixin(DataInterfaceTestMixin, TemporalAlignmentMixin): - """ - A mixin for testing DeepLabCut interfaces. - """ - - def check_interface_get_original_timestamps(self): - pass # TODO in separate PR - - def check_interface_get_timestamps(self): - pass # TODO in separate PR - - def check_interface_set_aligned_timestamps(self): - pass # TODO in separate PR - - def check_shift_timestamps_by_start_time(self): - pass # TODO in separate PR - - def check_interface_original_timestamps_inmutability(self): - pass # TODO in separate PR - - def check_nwbfile_temporal_alignment(self): - pass # TODO in separate PR - - class VideoInterfaceMixin(DataInterfaceTestMixin, TemporalAlignmentMixin): """ A mixin for testing Video interfaces. diff --git a/tests/test_on_data/behavior/test_behavior_interfaces.py b/tests/test_on_data/behavior/test_behavior_interfaces.py index 8e3e01d61..b43e65206 100644 --- a/tests/test_on_data/behavior/test_behavior_interfaces.py +++ b/tests/test_on_data/behavior/test_behavior_interfaces.py @@ -29,7 +29,6 @@ ) from neuroconv.tools.testing.data_interface_mixins import ( DataInterfaceTestMixin, - DeepLabCutInterfaceMixin, MedPCInterfaceMixin, TemporalAlignmentMixin, VideoInterfaceMixin, @@ -332,11 +331,16 @@ class TestFicTracDataInterfaceTiming(TemporalAlignmentMixin): platform == "darwin" and python_version < version.parse("3.10"), reason="interface not supported on macOS with Python < 3.10", ) -class TestDeepLabCutInterface(DeepLabCutInterfaceMixin): +class TestDeepLabCutInterface(DataInterfaceTestMixin): data_interface_cls = DeepLabCutInterface interface_kwargs = dict( - file_path=str(BEHAVIOR_DATA_PATH / "DLC" / "m3v1mp4DLC_resnet50_openfieldAug20shuffle1_30000.h5"), - config_file_path=str(BEHAVIOR_DATA_PATH / "DLC" / "config.yaml"), + file_path=str( + BEHAVIOR_DATA_PATH + / "DLC" + / "open_field_without_video" + / "m3v1mp4DLC_resnet50_openfieldAug20shuffle1_30000.h5" + ), + config_file_path=str(BEHAVIOR_DATA_PATH / "DLC" / "open_field_without_video" / "config.yaml"), subject_name="ind1", ) save_directory = OUTPUT_PATH @@ -384,7 +388,12 @@ def check_read_nwb(self, nwbfile_path: str): class TestDeepLabCutInterfaceNoConfigFile(DataInterfaceTestMixin): data_interface_cls = DeepLabCutInterface interface_kwargs = dict( - file_path=str(BEHAVIOR_DATA_PATH / "DLC" / "m3v1mp4DLC_resnet50_openfieldAug20shuffle1_30000.h5"), + file_path=str( + BEHAVIOR_DATA_PATH + / "DLC" + / "open_field_without_video" + / "m3v1mp4DLC_resnet50_openfieldAug20shuffle1_30000.h5" + ), config_file_path=None, subject_name="ind1", ) @@ -411,11 +420,16 @@ def check_read_nwb(self, nwbfile_path: str): platform == "darwin" and python_version < version.parse("3.10"), reason="interface not supported on macOS with Python < 3.10", ) -class TestDeepLabCutInterfaceSetTimestamps(DeepLabCutInterfaceMixin): +class TestDeepLabCutInterfaceSetTimestamps(DataInterfaceTestMixin): data_interface_cls = DeepLabCutInterface interface_kwargs = dict( - file_path=str(BEHAVIOR_DATA_PATH / "DLC" / "m3v1mp4DLC_resnet50_openfieldAug20shuffle1_30000.h5"), - config_file_path=str(BEHAVIOR_DATA_PATH / "DLC" / "config.yaml"), + file_path=str( + BEHAVIOR_DATA_PATH + / "DLC" + / "open_field_without_video" + / "m3v1mp4DLC_resnet50_openfieldAug20shuffle1_30000.h5" + ), + config_file_path=str(BEHAVIOR_DATA_PATH / "DLC" / "open_field_without_video" / "config.yaml"), subject_name="ind1", ) @@ -454,6 +468,41 @@ def check_read_nwb(self, nwbfile_path: str): pass +@pytest.mark.skipif( + platform == "darwin" and python_version < version.parse("3.10"), + reason="interface not supported on macOS with Python < 3.10", +) +class TestDeepLabCutInterfaceFromCSV(DataInterfaceTestMixin): + data_interface_cls = DeepLabCutInterface + interface_kwargs = dict( + file_path=str( + BEHAVIOR_DATA_PATH + / "DLC" + / "SL18_csv" + / "SL18_D19_S01_F01_BOX_SLP_20230503_112642.1DLC_resnet50_SubLearnSleepBoxRedLightJun26shuffle1_100000_stubbed.csv" + ), + config_file_path=None, + subject_name="SL18", + ) + save_directory = OUTPUT_PATH + + def check_read_nwb(self, nwbfile_path: str): + with NWBHDF5IO(path=nwbfile_path, mode="r", load_namespaces=True) as io: + nwbfile = io.read() + assert "behavior" in nwbfile.processing + processing_module_interfaces = nwbfile.processing["behavior"].data_interfaces + assert "PoseEstimation" in processing_module_interfaces + + pose_estimation_series_in_nwb = processing_module_interfaces["PoseEstimation"].pose_estimation_series + expected_pose_estimation_series = ["SL18_redled", "SL18_shoulder", "SL18_haunch", "SL18_baseoftail"] + + expected_pose_estimation_series_are_in_nwb_file = [ + pose_estimation in pose_estimation_series_in_nwb for pose_estimation in expected_pose_estimation_series + ] + + assert all(expected_pose_estimation_series_are_in_nwb_file) + + class TestSLEAPInterface(DataInterfaceTestMixin, TemporalAlignmentMixin): data_interface_cls = SLEAPInterface interface_kwargs = dict( From 64fb9e01a5f4070fd3b01ebab50d8fc19a2fe953 Mon Sep 17 00:00:00 2001 From: Heberto Mayorquin Date: Thu, 14 Nov 2024 21:33:52 -0600 Subject: [PATCH 5/6] Use mixing tests for mocks (#1136) --- CHANGELOG.md | 1 + .../tools/testing/data_interface_mixins.py | 1 - tests/test_ecephys/test_ecephys_interfaces.py | 114 +++++++----------- .../test_mock_recording_interface.py | 9 -- 4 files changed, 43 insertions(+), 82 deletions(-) delete mode 100644 tests/test_ecephys/test_mock_recording_interface.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 0545001d1..92f4e6b5f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ * Added .csv support to DeepLabCutInterface [PR #1140](https://github.com/catalystneuro/neuroconv/pull/1140) ## Improvements +* Use mixing tests for ecephy's mocks [PR #1136](https://github.com/catalystneuro/neuroconv/pull/1136) # v0.6.5 (November 1, 2024) diff --git a/src/neuroconv/tools/testing/data_interface_mixins.py b/src/neuroconv/tools/testing/data_interface_mixins.py index 5187ff2e4..fab049165 100644 --- a/src/neuroconv/tools/testing/data_interface_mixins.py +++ b/src/neuroconv/tools/testing/data_interface_mixins.py @@ -92,7 +92,6 @@ def test_metadata_schema_valid(self, setup_interface): Draft7Validator.check_schema(schema=schema) def test_metadata(self, setup_interface): - # Validate metadata now happens on the class itself metadata = self.interface.get_metadata() self.check_extracted_metadata(metadata) diff --git a/tests/test_ecephys/test_ecephys_interfaces.py b/tests/test_ecephys/test_ecephys_interfaces.py index 4d4232bf2..24393923f 100644 --- a/tests/test_ecephys/test_ecephys_interfaces.py +++ b/tests/test_ecephys/test_ecephys_interfaces.py @@ -27,42 +27,61 @@ python_version = Version(get_python_version()) +from neuroconv.tools.testing.data_interface_mixins import ( + RecordingExtractorInterfaceTestMixin, + SortingExtractorInterfaceTestMixin, +) -class TestRecordingInterface(TestCase): - @classmethod - def setUpClass(cls): - cls.single_segment_recording_interface = MockRecordingInterface(durations=[0.100]) - cls.multi_segment_recording_interface = MockRecordingInterface(durations=[0.100, 0.100]) - def test_stub_single_segment(self): - interface = self.single_segment_recording_interface +class TestSortingInterface(SortingExtractorInterfaceTestMixin): + + data_interface_cls = MockSortingInterface + interface_kwargs = dict(num_units=4, durations=[0.100]) + + def test_propagate_conversion_options(self, setup_interface): + interface = self.interface metadata = interface.get_metadata() - interface.create_nwbfile(stub_test=True, metadata=metadata) + nwbfile = interface.create_nwbfile( + stub_test=True, + metadata=metadata, + write_as="processing", + units_name="processed_units", + units_description="The processed units.", + ) - def test_stub_multi_segment(self): - interface = self.multi_segment_recording_interface + ecephys = get_module(nwbfile, "ecephys") + + assert nwbfile.units is None + assert "processed_units" in ecephys.data_interfaces + + +class TestRecordingInterface(RecordingExtractorInterfaceTestMixin): + data_interface_cls = MockRecordingInterface + interface_kwargs = dict(durations=[0.100]) + + def test_stub(self, setup_interface): + interface = self.interface metadata = interface.get_metadata() interface.create_nwbfile(stub_test=True, metadata=metadata) - def test_no_slash_in_name(self): - interface = self.single_segment_recording_interface + def test_no_slash_in_name(self, setup_interface): + interface = self.interface metadata = interface.get_metadata() metadata["Ecephys"]["ElectricalSeries"]["name"] = "test/slash" - with self.assertRaises(jsonschema.exceptions.ValidationError): + with pytest.raises(jsonschema.exceptions.ValidationError): interface.validate_metadata(metadata) + def test_stub_multi_segment(self): -class TestAlwaysWriteTimestamps: + interface = MockRecordingInterface(durations=[0.100, 0.100]) + metadata = interface.get_metadata() + interface.create_nwbfile(stub_test=True, metadata=metadata) - def test_always_write_timestamps(self): - # By default the MockRecordingInterface has a uniform sampling rate - interface = MockRecordingInterface(durations=[1.0], sampling_frequency=30_000.0) + def test_always_write_timestamps(self, setup_interface): - nwbfile = interface.create_nwbfile(always_write_timestamps=True) + nwbfile = self.interface.create_nwbfile(always_write_timestamps=True) electrical_series = nwbfile.acquisition["ElectricalSeries"] - - expected_timestamps = interface.recording_extractor.get_times() - + expected_timestamps = self.interface.recording_extractor.get_times() np.testing.assert_array_equal(electrical_series.timestamps[:], expected_timestamps) @@ -84,33 +103,9 @@ def test_spike2_import_assertions_3_11(self): Spike2RecordingInterface.get_all_channels_info(file_path="does_not_matter.smrx") -class TestSortingInterface: - - def test_run_conversion(self, tmp_path): - - nwbfile_path = Path(tmp_path) / "test_sorting.nwb" - num_units = 4 - interface = MockSortingInterface(num_units=num_units, durations=(1.0,)) - interface.sorting_extractor = interface.sorting_extractor.rename_units(new_unit_ids=["a", "b", "c", "d"]) - - interface.run_conversion(nwbfile_path=nwbfile_path) - with NWBHDF5IO(nwbfile_path, "r") as io: - nwbfile = io.read() - - units = nwbfile.units - assert len(units) == num_units - units_df = units.to_dataframe() - # Get index in units table - for unit_id in interface.sorting_extractor.unit_ids: - # In pynwb we write unit name as unit_id - row = units_df.query(f"unit_name == '{unit_id}'") - spike_times = interface.sorting_extractor.get_unit_spike_train(unit_id=unit_id, return_times=True) - written_spike_times = row["spike_times"].iloc[0] - - np.testing.assert_array_equal(spike_times, written_spike_times) - - class TestSortingInterfaceOld(unittest.TestCase): + """Old-style tests for the SortingInterface. Remove once we we are sure all the behaviors are covered by the mock.""" + @classmethod def setUpClass(cls) -> None: cls.test_dir = Path(mkdtemp()) @@ -194,28 +189,3 @@ def test_sorting_full(self): nwbfile = io.read() for i, start_times in enumerate(self.sorting_start_frames): assert len(nwbfile.units["spike_times"][i]) == self.num_frames - start_times - - def test_sorting_propagate_conversion_options(self): - minimal_nwbfile = self.test_dir / "temp2.nwb" - metadata = self.test_sorting_interface.get_metadata() - metadata["NWBFile"]["session_start_time"] = datetime.now().astimezone() - units_description = "The processed units." - conversion_options = dict( - TestSortingInterface=dict( - write_as="processing", - units_name="processed_units", - units_description=units_description, - ) - ) - self.test_sorting_interface.run_conversion( - nwbfile_path=minimal_nwbfile, - metadata=metadata, - conversion_options=conversion_options, - ) - - with NWBHDF5IO(minimal_nwbfile, "r") as io: - nwbfile = io.read() - ecephys = get_module(nwbfile, "ecephys") - self.assertIsNone(nwbfile.units) - self.assertIn("processed_units", ecephys.data_interfaces) - self.assertEqual(ecephys["processed_units"].description, units_description) diff --git a/tests/test_ecephys/test_mock_recording_interface.py b/tests/test_ecephys/test_mock_recording_interface.py deleted file mode 100644 index a33f3acd1..000000000 --- a/tests/test_ecephys/test_mock_recording_interface.py +++ /dev/null @@ -1,9 +0,0 @@ -from neuroconv.tools.testing.data_interface_mixins import ( - RecordingExtractorInterfaceTestMixin, -) -from neuroconv.tools.testing.mock_interfaces import MockRecordingInterface - - -class TestMockRecordingInterface(RecordingExtractorInterfaceTestMixin): - data_interface_cls = MockRecordingInterface - interface_kwargs = dict(durations=[0.100]) From a608e904ad6fd75f7f983c1555bea1832f850f8e Mon Sep 17 00:00:00 2001 From: Heberto Mayorquin Date: Fri, 15 Nov 2024 12:24:14 -0600 Subject: [PATCH 6/6] Propagate `unit_electrode_indices` to `SortingInterface` (#1124) --- CHANGELOG.md | 1 + .../ecephys/basesortingextractorinterface.py | 8 +++++ .../tools/spikeinterface/spikeinterface.py | 19 +++++++--- tests/test_ecephys/test_ecephys_interfaces.py | 35 +++++++++++++++++++ 4 files changed, 58 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 92f4e6b5f..002aed660 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ * datetime objects now can be validated as conversion options [#1139](https://github.com/catalystneuro/neuroconv/pull/1126) ## Features +* Propagate the `unit_electrode_indices` argument from the spikeinterface tools to `BaseSortingExtractorInterface`. This allows users to map units to the electrode table when adding sorting data [PR #1124](https://github.com/catalystneuro/neuroconv/pull/1124) * Imaging interfaces have a new conversion option `always_write_timestamps` that can be used to force writing timestamps even if neuroconv's heuristics indicates regular sampling rate [PR #1125](https://github.com/catalystneuro/neuroconv/pull/1125) * Added .csv support to DeepLabCutInterface [PR #1140](https://github.com/catalystneuro/neuroconv/pull/1140) diff --git a/src/neuroconv/datainterfaces/ecephys/basesortingextractorinterface.py b/src/neuroconv/datainterfaces/ecephys/basesortingextractorinterface.py index cd8396154..dca2dea5f 100644 --- a/src/neuroconv/datainterfaces/ecephys/basesortingextractorinterface.py +++ b/src/neuroconv/datainterfaces/ecephys/basesortingextractorinterface.py @@ -288,6 +288,7 @@ def add_to_nwbfile( write_as: Literal["units", "processing"] = "units", units_name: str = "units", units_description: str = "Autogenerated by neuroconv.", + unit_electrode_indices: Optional[list[list[int]]] = None, ): """ Primary function for converting the data in a SortingExtractor to NWB format. @@ -312,9 +313,15 @@ def add_to_nwbfile( units_name : str, default: 'units' The name of the units table. If write_as=='units', then units_name must also be 'units'. units_description : str, default: 'Autogenerated by neuroconv.' + unit_electrode_indices : list of lists of int, optional + A list of lists of integers indicating the indices of the electrodes that each unit is associated with. + The length of the list must match the number of units in the sorting extractor. """ from ...tools.spikeinterface import add_sorting_to_nwbfile + if metadata is None: + metadata = self.get_metadata() + metadata_copy = deepcopy(metadata) if write_ecephys_metadata: self.add_channel_metadata_to_nwb(nwbfile=nwbfile, metadata=metadata_copy) @@ -346,4 +353,5 @@ def add_to_nwbfile( write_as=write_as, units_name=units_name, units_description=units_description, + unit_electrode_indices=unit_electrode_indices, ) diff --git a/src/neuroconv/tools/spikeinterface/spikeinterface.py b/src/neuroconv/tools/spikeinterface/spikeinterface.py index 5aa3c8925..fa00d58ed 100644 --- a/src/neuroconv/tools/spikeinterface/spikeinterface.py +++ b/src/neuroconv/tools/spikeinterface/spikeinterface.py @@ -1368,7 +1368,7 @@ def add_units_table_to_nwbfile( write_in_processing_module: bool = False, waveform_means: Optional[np.ndarray] = None, waveform_sds: Optional[np.ndarray] = None, - unit_electrode_indices=None, + unit_electrode_indices: Optional[list[list[int]]] = None, null_values_for_properties: Optional[dict] = None, ): """ @@ -1405,8 +1405,9 @@ def add_units_table_to_nwbfile( Waveform standard deviation for each unit. Shape: (num_units, num_samples, num_channels). unit_electrode_indices : list of lists of int, optional For each unit, a list of electrode indices corresponding to waveform data. - null_values_for_properties: dict, optional - A dictionary mapping properties to null values to use when the property is not present + unit_electrode_indices : list of lists of int, optional + A list of lists of integers indicating the indices of the electrodes that each unit is associated with. + The length of the list must match the number of units in the sorting extractor. """ unit_table_description = unit_table_description or "Autogenerated by neuroconv." @@ -1414,6 +1415,13 @@ def add_units_table_to_nwbfile( nwbfile, pynwb.NWBFile ), f"'nwbfile' should be of type pynwb.NWBFile but is of type {type(nwbfile)}" + if unit_electrode_indices is not None: + electrodes_table = nwbfile.electrodes + if electrodes_table is None: + raise ValueError( + "Electrodes table is required to map units to electrodes. Add an electrode table to the NWBFile first." + ) + null_values_for_properties = dict() if null_values_for_properties is None else null_values_for_properties if not write_in_processing_module and units_table_name != "units": @@ -1668,7 +1676,7 @@ def add_sorting_to_nwbfile( units_description: str = "Autogenerated by neuroconv.", waveform_means: Optional[np.ndarray] = None, waveform_sds: Optional[np.ndarray] = None, - unit_electrode_indices=None, + unit_electrode_indices: Optional[list[list[int]]] = None, ): """Add sorting data (units and their properties) to an NWBFile. @@ -1703,7 +1711,8 @@ def add_sorting_to_nwbfile( waveform_sds : np.ndarray, optional Waveform standard deviation for each unit. Shape: (num_units, num_samples, num_channels). unit_electrode_indices : list of lists of int, optional - For each unit, a list of electrode indices corresponding to waveform data. + A list of lists of integers indicating the indices of the electrodes that each unit is associated with. + The length of the list must match the number of units in the sorting extractor. """ if skip_features is not None: diff --git a/tests/test_ecephys/test_ecephys_interfaces.py b/tests/test_ecephys/test_ecephys_interfaces.py index 24393923f..e036ccb81 100644 --- a/tests/test_ecephys/test_ecephys_interfaces.py +++ b/tests/test_ecephys/test_ecephys_interfaces.py @@ -54,6 +54,41 @@ def test_propagate_conversion_options(self, setup_interface): assert nwbfile.units is None assert "processed_units" in ecephys.data_interfaces + def test_electrode_indices(self, setup_interface): + + recording_interface = MockRecordingInterface(num_channels=4, durations=[0.100]) + recording_extractor = recording_interface.recording_extractor + recording_extractor = recording_extractor.rename_channels(new_channel_ids=["a", "b", "c", "d"]) + recording_extractor.set_property(key="property", values=["A", "B", "C", "D"]) + recording_interface.recording_extractor = recording_extractor + + nwbfile = recording_interface.create_nwbfile() + + unit_electrode_indices = [[3], [0, 1], [1], [2]] + expected_properties_matching = [["D"], ["A", "B"], ["B"], ["C"]] + self.interface.add_to_nwbfile(nwbfile=nwbfile, unit_electrode_indices=unit_electrode_indices) + + unit_table = nwbfile.units + + for unit_row, electrode_indices, property in zip( + unit_table.to_dataframe().itertuples(index=False), + unit_electrode_indices, + expected_properties_matching, + ): + electrode_table_region = unit_row.electrodes + electrode_table_region_indices = electrode_table_region.index.to_list() + assert electrode_table_region_indices == electrode_indices + + electrode_table_region_properties = electrode_table_region["property"].to_list() + assert electrode_table_region_properties == property + + def test_electrode_indices_assertion_error_when_missing_table(self, setup_interface): + with pytest.raises( + ValueError, + match="Electrodes table is required to map units to electrodes. Add an electrode table to the NWBFile first.", + ): + self.interface.create_nwbfile(unit_electrode_indices=[[0], [1], [2], [3]]) + class TestRecordingInterface(RecordingExtractorInterfaceTestMixin): data_interface_cls = MockRecordingInterface