From 1ff16ed5ced47065b056ccd59612b78f9840fb8d Mon Sep 17 00:00:00 2001 From: Steph Prince <40640337+stephprince@users.noreply.github.com> Date: Wed, 4 Jun 2025 15:53:52 -0700 Subject: [PATCH 1/8] validate source_idx inputs for EventDetection --- src/pynwb/ecephys.py | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/src/pynwb/ecephys.py b/src/pynwb/ecephys.py index 96db7c1a0..a0f8d4b06 100644 --- a/src/pynwb/ecephys.py +++ b/src/pynwb/ecephys.py @@ -220,15 +220,31 @@ class EventDetection(NWBDataInterface): {'name': 'source_electricalseries', 'type': ElectricalSeries, 'doc': 'The source electrophysiology data'}, {'name': 'source_idx', 'type': ('array_data', 'data'), 'doc': 'Indices (zero-based) into source ElectricalSeries::data array corresponding ' - 'to time of event. Module description should define what is meant by time of event ' - '(e.g., .25msec before action potential peak, zero-crossing time, etc). ' + 'to time of event or time and channel of event. For 1D arrays, specifies the time ' + 'index for each event. For 2D arrays with shape (num_events, 2), specifies ' + '[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': '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) + + # Validate source_idx shape + source_idx = args_to_set['source_idx'] + source_idx_shape = get_data_shape(source_idx, strict_no_data_load=True) + if source_idx_shape is not None: + if len(source_idx_shape) == 2: + # 2D array must have shape (num_events, 2) for [time_index, channel_index] + if source_idx_shape[1] != 2: + raise ValueError(f"EventDetection source_idx: 2D source_idx must have shape (num_events, 2) " + f"for [time_index, channel_index], but got shape {source_idx_shape}") + elif len(source_idx_shape) > 2: + raise ValueError(f"EventDetection source_idx: source_idx must be 1D or 2D array, " + f"but got {len(source_idx_shape)}D array with shape {source_idx_shape}") + for key, val in args_to_set.items(): setattr(self, key, val) self.unit = 'seconds' # fixed value From b83f090b08ad8f45799a8ba359b9bbc671b36e9f Mon Sep 17 00:00:00 2001 From: Steph Prince <40640337+stephprince@users.noreply.github.com> Date: Wed, 4 Jun 2025 16:00:05 -0700 Subject: [PATCH 2/8] add tests for EventDetection schema updates --- tests/unit/test_ecephys.py | 73 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 73 insertions(+) diff --git a/tests/unit/test_ecephys.py b/tests/unit/test_ecephys.py index cffebdb59..7dcefd534 100644 --- a/tests/unit/test_ecephys.py +++ b/tests/unit/test_ecephys.py @@ -290,6 +290,79 @@ def test_init(self): self.assertEqual(eD.times, (0.1, 0.2, 0.3)) self.assertEqual(eD.unit, 'seconds') + def test_init_2d_source_idx(self): + """Test EventDetection with 2D source_idx containing time and channel indices""" + data = np.random.rand(10, 2) # 10 time points, 4 channels + 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) + + # 2D source_idx with shape (num_events, 2) for [time_index, channel_index] + source_idx_2d = np.array([[1, 0], [2, 1], [3, 0],]) # 4 events + times = (0.1, 0.2, 0.3) # Corresponding times for the events + + eD = EventDetection(detection_method='threshold detection', + source_electricalseries=eS, + source_idx=source_idx_2d, + times=times) + + 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): + """Test EventDetection with optional times parameter (times=None)""" + 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 with times=None (should not set unit attribute) + eD = EventDetection(detection_method='detection_method', + source_electricalseries=eS, + 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.assertIsNone(eD.times) + + def test_invalid_2d_source_idx_shape(self): + """Test error handling for invalid 2D source_idx shapes""" + 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 with invalid 2D shape (num_events, 3) - should be (num_events, 2) + invalid_source_idx = np.array([[1, 0, 5], [2, 1, 6]]) + + msg = (f"EventDetection source_idx: 2D source_idx must have shape (num_events, 2) " + f"for [time_index, channel_index], but got shape {invalid_source_idx.shape}") + with self.assertRaisesWith(ValueError, msg): + EventDetection(detection_method='detection_method', + source_electricalseries=eS, + source_idx=invalid_source_idx, + times=(0.1, 0.2)) + + def test_invalid_3d_source_idx(self): + """Test error handling for 3D source_idx arrays""" + 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 with 3D array - should fail + invalid_source_idx = np.array([[[1, 0], [2, 1]], [[3, 0], [4, 1]]]) + + msg = (f"EventDetection source_idx: source_idx must be 1D or 2D array, " + f"but got {len(invalid_source_idx.shape)}D array with shape {invalid_source_idx.shape}") + with self.assertRaisesWith(ValueError, msg): + EventDetection(detection_method='detection_method', + source_electricalseries=eS, + source_idx=invalid_source_idx, + times=(0.1, 0.2)) class EventWaveformConstructor(TestCase): From 7235e5efa19c38afc7198cfa53229adfe0da816d Mon Sep 17 00:00:00 2001 From: Steph Prince <40640337+stephprince@users.noreply.github.com> Date: Wed, 4 Jun 2025 16:00:43 -0700 Subject: [PATCH 3/8] add 2D source index example for EventDetection --- docs/gallery/domain/ecephys.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/gallery/domain/ecephys.py b/docs/gallery/domain/ecephys.py index 11e39b44a..8683a72c8 100644 --- a/docs/gallery/domain/ecephys.py +++ b/docs/gallery/domain/ecephys.py @@ -381,7 +381,7 @@ name="threshold_events", detection_method="thresholding, 1.5 * std", source_electricalseries=raw_electrical_series, - source_idx=[1000, 2000, 3000], + source_idx=[[1000, 0], [2000, 4], [3000, 8]], # indicates the event and channel indices times=[.033, .066, .099], ) From 55830535b54a6a9bf270cfa0a950d7db4934c185 Mon Sep 17 00:00:00 2001 From: Steph Prince <40640337+stephprince@users.noreply.github.com> Date: Wed, 4 Jun 2025 16:42:41 -0700 Subject: [PATCH 4/8] fix formatting and comments --- src/pynwb/ecephys.py | 3 ++- tests/unit/test_ecephys.py | 10 +++++----- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/pynwb/ecephys.py b/src/pynwb/ecephys.py index a0f8d4b06..b86da1748 100644 --- a/src/pynwb/ecephys.py +++ b/src/pynwb/ecephys.py @@ -225,7 +225,8 @@ 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', 'default': None}, + {'name': 'times', 'type': ('array_data', 'data'), 'doc': '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): diff --git a/tests/unit/test_ecephys.py b/tests/unit/test_ecephys.py index 7dcefd534..81634ca50 100644 --- a/tests/unit/test_ecephys.py +++ b/tests/unit/test_ecephys.py @@ -292,14 +292,14 @@ def test_init(self): def test_init_2d_source_idx(self): """Test EventDetection with 2D source_idx containing time and channel indices""" - data = np.random.rand(10, 2) # 10 time points, 4 channels + data = np.random.rand(10, 2) # 10 time points, 2 channels 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) # 2D source_idx with shape (num_events, 2) for [time_index, channel_index] - source_idx_2d = np.array([[1, 0], [2, 1], [3, 0],]) # 4 events - times = (0.1, 0.2, 0.3) # Corresponding times for the events + 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, @@ -319,7 +319,6 @@ def test_init_optional_times(self): _, region = self._create_table_and_region() eS = ElectricalSeries(name='test_eS', data=data, electrodes=region, timestamps=ts) - # Test with times=None (should not set unit attribute) eD = EventDetection(detection_method='detection_method', source_electricalseries=eS, source_idx=(1, 2, 3)) @@ -353,7 +352,8 @@ def test_invalid_3d_source_idx(self): 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 with 3D array - should fail + + # test with 3D array - should raise ValueError invalid_source_idx = np.array([[[1, 0], [2, 1]], [[3, 0], [4, 1]]]) msg = (f"EventDetection source_idx: source_idx must be 1D or 2D array, " From c43eb4f1027eec5a1cfb1cbb01be2481b2c88922 Mon Sep 17 00:00:00 2001 From: Steph Prince <40640337+stephprince@users.noreply.github.com> Date: Wed, 4 Jun 2025 16:47:38 -0700 Subject: [PATCH 5/8] update comments --- docs/gallery/domain/ecephys.py | 2 +- src/pynwb/ecephys.py | 10 ++++------ 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/docs/gallery/domain/ecephys.py b/docs/gallery/domain/ecephys.py index 8683a72c8..940a31938 100644 --- a/docs/gallery/domain/ecephys.py +++ b/docs/gallery/domain/ecephys.py @@ -381,7 +381,7 @@ name="threshold_events", detection_method="thresholding, 1.5 * std", source_electricalseries=raw_electrical_series, - source_idx=[[1000, 0], [2000, 4], [3000, 8]], # indicates the event and channel indices + source_idx=[[1000, 0], [2000, 4], [3000, 8]], # indicates the time and channel indices times=[.033, .066, .099], ) diff --git a/src/pynwb/ecephys.py b/src/pynwb/ecephys.py index b86da1748..c6b7c77c5 100644 --- a/src/pynwb/ecephys.py +++ b/src/pynwb/ecephys.py @@ -237,14 +237,12 @@ def __init__(self, **kwargs): source_idx = args_to_set['source_idx'] source_idx_shape = get_data_shape(source_idx, strict_no_data_load=True) if source_idx_shape is not None: - if len(source_idx_shape) == 2: - # 2D array must have shape (num_events, 2) for [time_index, channel_index] - if source_idx_shape[1] != 2: - raise ValueError(f"EventDetection source_idx: 2D source_idx must have shape (num_events, 2) " - f"for [time_index, channel_index], but got shape {source_idx_shape}") + if len(source_idx_shape) == 2 and source_idx_shape[1] != 2: + raise ValueError(f"EventDetection source_idx: 2D source_idx must have shape (num_events, 2) " + f"for [time_index, channel_index], but got shape {source_idx_shape}") elif len(source_idx_shape) > 2: raise ValueError(f"EventDetection source_idx: source_idx must be 1D or 2D array, " - f"but got {len(source_idx_shape)}D array with shape {source_idx_shape}") + f"but got {len(source_idx_shape)}D array with shape {source_idx_shape}") for key, val in args_to_set.items(): setattr(self, key, val) From 824cb7d45c1e4e967a893ffd6ec39d6c9d5ce315 Mon Sep 17 00:00:00 2001 From: Steph Prince <40640337+stephprince@users.noreply.github.com> Date: Wed, 4 Jun 2025 16:51:51 -0700 Subject: [PATCH 6/8] update CHANGELOG --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8b4d94a1b..77907e2fb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,8 @@ - Added new `ElectrodesTable` neurodata type. @mavaylon1 [#1890](https://github.com/NeurodataWithoutBorders/pynwb/pull/1890) - Formally defined and renamed `ElectrodeTable` as the `ElectrodesTable` neurodata type. @mavaylon1 [#1890](https://github.com/NeurodataWithoutBorders/pynwb/pull/1890) - Formally defined bands within `DecompositionSeries` as the neurodatatype `FrequencyBandsTable`. @mavaylon1 @rly [#2063](https://github.com/NeurodataWithoutBorders/pynwb/pull/2063) + - 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) - 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) From 8723d3f0910a6a02d823d5e6a6efdf803f632c75 Mon Sep 17 00:00:00 2001 From: Steph Prince <40640337+stephprince@users.noreply.github.com> Date: Wed, 4 Jun 2025 16:52:28 -0700 Subject: [PATCH 7/8] point to event detection schema branch --- src/pynwb/nwb-schema | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pynwb/nwb-schema b/src/pynwb/nwb-schema index 362c09585..2036b6fa8 160000 --- a/src/pynwb/nwb-schema +++ b/src/pynwb/nwb-schema @@ -1 +1 @@ -Subproject commit 362c0958528868ac323b1d986ecba5050d0cbd36 +Subproject commit 2036b6fa81b4f42c1f6c849e92eac61502e16c67 From d35ecf142a068678fc545940a80636af682f3c23 Mon Sep 17 00:00:00 2001 From: rly Date: Mon, 9 Jun 2025 13:38:00 -0700 Subject: [PATCH 8/8] Update schema submodule --- src/pynwb/nwb-schema | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pynwb/nwb-schema b/src/pynwb/nwb-schema index 2036b6fa8..97f3ead4d 160000 --- a/src/pynwb/nwb-schema +++ b/src/pynwb/nwb-schema @@ -1 +1 @@ -Subproject commit 2036b6fa81b4f42c1f6c849e92eac61502e16c67 +Subproject commit 97f3ead4d01f09e32a6b9143d339c1c7f67dc32f