From cf100d484f0fc0c9e28cb6fc64d612783571bfd5 Mon Sep 17 00:00:00 2001 From: Steph Prince <40640337+stephprince@users.noreply.github.com> Date: Thu, 12 Jun 2025 17:25:20 -0700 Subject: [PATCH 1/3] add deprecation warning to eventdetection --- src/pynwb/ecephys.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/pynwb/ecephys.py b/src/pynwb/ecephys.py index c6b7c77c5..1c781de8a 100644 --- a/src/pynwb/ecephys.py +++ b/src/pynwb/ecephys.py @@ -225,7 +225,7 @@ class EventDetection(NWBDataInterface): '[time_index, channel_index] for each event. Module description should define what is meant ' 'by time of event (e.g., .25msec before action potential peak, zero-crossing time, etc). ' 'The index points to each event from the raw data'}, - {'name': 'times', 'type': ('array_data', 'data'), 'doc': 'Timestamps of events, in Seconds', + {'name': 'times', 'type': ('array_data', 'data'), 'doc': 'DEPRECATED. Timestamps of events, in Seconds', 'default': None}, {'name': 'name', 'type': str, 'doc': 'the name of this container', 'default': 'EventDetection'}, allow_positional=AllowPositional.WARNING,) @@ -233,6 +233,13 @@ def __init__(self, **kwargs): args_to_set = popargs_to_dict(('detection_method', 'source_electricalseries', 'source_idx', 'times'), kwargs) super().__init__(**kwargs) + if args_to_set['times'] is not None: + warnings.warn( + "The 'times' argument is deprecated and will be removed in a future version. " \ + "Use 'source_idx' instead to specify the time of events.", + DeprecationWarning, + ) + # Validate source_idx shape source_idx = args_to_set['source_idx'] source_idx_shape = get_data_shape(source_idx, strict_no_data_load=True) From 186bb03ef89fe66292f2f531c8a07cd0a16c32c8 Mon Sep 17 00:00:00 2001 From: Steph Prince <40640337+stephprince@users.noreply.github.com> Date: Thu, 12 Jun 2025 17:27:53 -0700 Subject: [PATCH 2/3] add tests for time deprecation --- tests/integration/hdf5/test_ecephys.py | 1 - tests/unit/test_ecephys.py | 31 +++++++++++++++++--------- 2 files changed, 20 insertions(+), 12 deletions(-) diff --git a/tests/integration/hdf5/test_ecephys.py b/tests/integration/hdf5/test_ecephys.py index 0ad27cb1b..1b0872fc0 100644 --- a/tests/integration/hdf5/test_ecephys.py +++ b/tests/integration/hdf5/test_ecephys.py @@ -326,7 +326,6 @@ def addContainer(self): detection_method='detection_method', source_electricalseries=eS, source_idx=(1, 2, 3), - times=(0.1, 0.2, 0.3) ) self.nwbfile.add_acquisition(eS) diff --git a/tests/unit/test_ecephys.py b/tests/unit/test_ecephys.py index 81634ca50..8ee4abac4 100644 --- a/tests/unit/test_ecephys.py +++ b/tests/unit/test_ecephys.py @@ -282,12 +282,10 @@ def test_init(self): eS = ElectricalSeries(name='test_eS', data=data, electrodes=region, timestamps=ts) eD = EventDetection(detection_method='detection_method', source_electricalseries=eS, - source_idx=(1, 2, 3), - times=(0.1, 0.2, 0.3)) + source_idx=(1, 2, 3)) self.assertEqual(eD.detection_method, 'detection_method') self.assertEqual(eD.source_electricalseries, eS) self.assertEqual(eD.source_idx, (1, 2, 3)) - self.assertEqual(eD.times, (0.1, 0.2, 0.3)) self.assertEqual(eD.unit, 'seconds') def test_init_2d_source_idx(self): @@ -299,17 +297,14 @@ def test_init_2d_source_idx(self): # 2D source_idx with shape (num_events, 2) for [time_index, channel_index] source_idx_2d = np.array([[1, 0], [2, 1], [3, 0],]) # 3 events - times = (0.1, 0.2, 0.3) eD = EventDetection(detection_method='threshold detection', source_electricalseries=eS, - source_idx=source_idx_2d, - times=times) + source_idx=source_idx_2d) self.assertEqual(eD.detection_method, 'threshold detection') self.assertEqual(eD.source_electricalseries, eS) np.testing.assert_array_equal(eD.source_idx, source_idx_2d) - self.assertEqual(eD.times, times) self.assertEqual(eD.unit, 'seconds') def test_init_optional_times(self): @@ -328,6 +323,22 @@ def test_init_optional_times(self): self.assertEqual(eD.source_idx, (1, 2, 3)) self.assertIsNone(eD.times) + def test_times_deprecation_warning(self): + """Test that using times parameter raises deprecation warning""" + data = list(range(10)) + ts = [0.1, 0.2, 0.3, 0.4, 0.5, 0.6, 0.7, 0.8, 0.9, 1.0] + _, region = self._create_table_and_region() + eS = ElectricalSeries(name='test_eS', data=data, electrodes=region, timestamps=ts) + + # Test that deprecation warning is raised when times is provided + msg = ("The 'times' argument is deprecated and will be removed in a future version. " + "Use 'source_idx' instead to specify the time of events.") + with self.assertWarnsWith(DeprecationWarning, msg): + EventDetection(detection_method='test_method', + source_electricalseries=eS, + source_idx=(1, 2, 3), + times=(0.1, 0.2, 0.3)) + def test_invalid_2d_source_idx_shape(self): """Test error handling for invalid 2D source_idx shapes""" data = list(range(10)) @@ -343,8 +354,7 @@ def test_invalid_2d_source_idx_shape(self): with self.assertRaisesWith(ValueError, msg): EventDetection(detection_method='detection_method', source_electricalseries=eS, - source_idx=invalid_source_idx, - times=(0.1, 0.2)) + source_idx=invalid_source_idx) def test_invalid_3d_source_idx(self): """Test error handling for 3D source_idx arrays""" @@ -361,8 +371,7 @@ def test_invalid_3d_source_idx(self): with self.assertRaisesWith(ValueError, msg): EventDetection(detection_method='detection_method', source_electricalseries=eS, - source_idx=invalid_source_idx, - times=(0.1, 0.2)) + source_idx=invalid_source_idx) class EventWaveformConstructor(TestCase): From 39f5851a9f30567ee33cc5e204d10ab92b2119c8 Mon Sep 17 00:00:00 2001 From: Steph Prince <40640337+stephprince@users.noreply.github.com> Date: Thu, 12 Jun 2025 17:32:33 -0700 Subject: [PATCH 3/3] update CHANGELOG --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 40cc5d252..606d6abe2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ - Deprecated `Device.model_name`, `Device.model_number`, and `Device.manufacturer` fields in favor of `DeviceModel`. @rly [#2088](https://github.com/NeurodataWithoutBorders/pynwb/pull/2088) - Added support for 2D `EventDetection.source_index` to indicate [time_index, channel_index]. @stephprince [#2091](https://github.com/NeurodataWithoutBorders/pynwb/pull/2091) - Made `EventDetection.times` optional. @stephprince [#2091](https://github.com/NeurodataWithoutBorders/pynwb/pull/2091) + - Deprecated `EventDetection.times`. @stephprince [#2101](https://github.com/NeurodataWithoutBorders/pynwb/pull/2101) - Automatically add timezone information to timestamps reference time if no timezone information is specified. @stephprince [#2056](https://github.com/NeurodataWithoutBorders/pynwb/pull/2056) - Added option to disable typemap caching and updated type map cache location. @stephprince [#2057](https://github.com/NeurodataWithoutBorders/pynwb/pull/2057) - Added dictionary-like operations directly on `ProcessingModule` objects (e.g., `len(processing_module)`). @bendichter [#2020](https://github.com/NeurodataWithoutBorders/pynwb/pull/2020)