Deprecate EventDetection.times (#2101)

* add deprecation warning to eventdetection

* add tests for time deprecation

* update CHANGELOG

Author: stephprince

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)

src/pynwb/ecephys.py

@@ -225,14 +225,21 @@ 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,)
     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)

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)

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):