From 2d74acb4de2dd9a8032f6c2b6f3aad8d5728fd27 Mon Sep 17 00:00:00 2001 From: rly Date: Tue, 23 Jul 2024 14:39:49 -0700 Subject: [PATCH 1/2] Deprecate EventWaveform class --- docs/gallery/domain/ecephys.py | 3 +-- docs/gallery/general/plot_file.py | 4 +--- src/pynwb/ecephys.py | 11 ++++++++--- src/pynwb/nwb-schema | 2 +- tests/integration/hdf5/test_ecephys.py | 11 ++++------- tests/unit/test_ecephys.py | 22 ++-------------------- 6 files changed, 17 insertions(+), 36 deletions(-) diff --git a/docs/gallery/domain/ecephys.py b/docs/gallery/domain/ecephys.py index 3f239eb77..d69a94c5b 100644 --- a/docs/gallery/domain/ecephys.py +++ b/docs/gallery/domain/ecephys.py @@ -288,8 +288,7 @@ # :py:class:`~pynwb.ecephys.ElectricalSeries` objects as :ref:`acquisition ` data, and use # the :py:class:`~pynwb.ecephys.EventDetection` class for identifying the spike events in your raw traces. # If you do not want to store the raw voltage traces and only the waveform 'snippets' surrounding spike events, -# you should use the :py:class:`~pynwb.ecephys.EventWaveform` class, which can store one or more -# :py:class:`~pynwb.ecephys.SpikeEventSeries` objects. +# you should store the snippets with :py:class:`~pynwb.ecephys.SpikeEventSeries` objects. # # The results of spike sorting (or clustering) should be stored in the top-level :py:class:`~pynwb.misc.Units` table. # Note that it is not required to store spike waveforms in order to store spike events or mean waveforms--if you only diff --git a/docs/gallery/general/plot_file.py b/docs/gallery/general/plot_file.py index b410d4b69..23769616c 100644 --- a/docs/gallery/general/plot_file.py +++ b/docs/gallery/general/plot_file.py @@ -92,7 +92,6 @@ :py:class:`~pynwb.behavior.EyeTracking`. * **Extracellular electrophysiology:** :py:class:`~pynwb.ecephys.EventDetection`, - :py:class:`~pynwb.ecephys.EventWaveform`, :py:class:`~pynwb.ecephys.FeatureExtraction`, :py:class:`~pynwb.ecephys.FilteredEphys`, :py:class:`~pynwb.ecephys.LFP`. @@ -571,8 +570,7 @@ #################### # .. [#] Some data interface objects have a default name. This default name is the type of the data interface. For -# example, the default name for :py:class:`~pynwb.ophys.ImageSegmentation` is "ImageSegmentation" and the default -# name for :py:class:`~pynwb.ecephys.EventWaveform` is "EventWaveform". +# example, the default name for :py:class:`~pynwb.ophys.ImageSegmentation` is "ImageSegmentation". # # .. [#] HDF5 is the primary backend supported by NWB. # diff --git a/src/pynwb/ecephys.py b/src/pynwb/ecephys.py index 3187cca4a..126667efc 100644 --- a/src/pynwb/ecephys.py +++ b/src/pynwb/ecephys.py @@ -101,8 +101,7 @@ class SpikeEventSeries(ElectricalSeries): """ Stores "snapshots" of spike events (i.e., threshold crossings) in data. This may also be raw data, as reported by ephys hardware. If so, the TimeSeries::description field should describing how - events were detected. All SpikeEventSeries should reside in a module (under EventWaveform - interface) even if the spikes were reported and stored by hardware. All events span the same + events were detected. All events span the same recording channels and store snapshots of equal duration. TimeSeries::data array structure: [num events] [num channels] [num samples] (or [num events] [num samples] for single electrode). @@ -162,7 +161,7 @@ def __init__(self, **kwargs): @register_class('EventWaveform', CORE_NAMESPACE) class EventWaveform(MultiContainerInterface): """ - Spike data for spike events detected in raw data + DEPRECATED. Spike data for spike events detected in raw data stored in this NWBFile, or events detect at acquisition """ @@ -174,6 +173,12 @@ class EventWaveform(MultiContainerInterface): 'create': 'create_spike_event_series' } + def __init__(self, **kwargs): + raise ValueError( + "This neurodata type is deprecated. If you are interested in using it, " + "please create an issue on https://github.com/NeurodataWithoutBorders/nwb-schema/issues." + ) + @register_class('Clustering', CORE_NAMESPACE) class Clustering(NWBDataInterface): diff --git a/src/pynwb/nwb-schema b/src/pynwb/nwb-schema index d65d42257..041e48534 160000 --- a/src/pynwb/nwb-schema +++ b/src/pynwb/nwb-schema @@ -1 +1 @@ -Subproject commit d65d42257003543c569ea7ac0cd6d7aee01c88d6 +Subproject commit 041e48534da072caee475bb044da1a6c0991e0ee diff --git a/tests/integration/hdf5/test_ecephys.py b/tests/integration/hdf5/test_ecephys.py index ff67d27c9..a305e4908 100644 --- a/tests/integration/hdf5/test_ecephys.py +++ b/tests/integration/hdf5/test_ecephys.py @@ -9,7 +9,6 @@ Clustering, ClusterWaveforms, SpikeEventSeries, - EventWaveform, EventDetection, FeatureExtraction, ) @@ -184,7 +183,7 @@ def roundtripExportContainer(self, cache_spec=False): return super().roundtripExportContainer(cache_spec) -class EventWaveformConstructor(NWBH5IOFlexMixin, TestCase): +class SpikeEventSeriesConstructor(NWBH5IOFlexMixin, TestCase): def getContainerType(self): return "SpikeEventSeries" @@ -201,18 +200,16 @@ def addContainer(self): description='the first and third electrodes', table=table) ses = SpikeEventSeries( - name='test_sES', + name='SpikeEventSeries', data=((1, 1), (2, 2), (3, 3)), timestamps=[0., 1., 2.], electrodes=region ) - ew = EventWaveform() - self.nwbfile.add_acquisition(ew) - ew.add_spike_event_series(ses) + self.nwbfile.add_acquisition(ses) def getContainer(self, nwbfile: NWBFile): - return nwbfile.acquisition['EventWaveform'] + return nwbfile.acquisition['SpikeEventSeries'] class ClusterWaveformsConstructor(AcquisitionH5IOMixin, TestCase): diff --git a/tests/unit/test_ecephys.py b/tests/unit/test_ecephys.py index dc194af2a..6e543b559 100644 --- a/tests/unit/test_ecephys.py +++ b/tests/unit/test_ecephys.py @@ -227,27 +227,9 @@ def test_init(self): class EventWaveformConstructor(TestCase): - def _create_table_and_region(self): - table = make_electrode_table() - region = DynamicTableRegion( - name='electrodes', - data=[0, 2], - description='the first and third electrodes', - table=table - ) - return table, region - def test_init(self): - table, region = self._create_table_and_region() - sES = SpikeEventSeries('test_sES', list(range(10)), list(range(10)), region) - - pm = ProcessingModule(name='test_module', description='a test module') - ew = EventWaveform() - pm.add(table) - pm.add(ew) - ew.add_spike_event_series(sES) - self.assertEqual(ew.spike_event_series['test_sES'], sES) - self.assertEqual(ew['test_sES'], ew.spike_event_series['test_sES']) + with self.assertRaises(ValueError): + EventWaveform() class ClusteringConstructor(TestCase): From 80753fa29e389c9aca6b846048df8ad1f516bdf7 Mon Sep 17 00:00:00 2001 From: Ryan Ly Date: Thu, 19 Sep 2024 22:16:26 -0700 Subject: [PATCH 2/2] Update ecephys.py --- src/pynwb/ecephys.py | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/pynwb/ecephys.py b/src/pynwb/ecephys.py index 126667efc..ad8add73c 100644 --- a/src/pynwb/ecephys.py +++ b/src/pynwb/ecephys.py @@ -161,7 +161,8 @@ def __init__(self, **kwargs): @register_class('EventWaveform', CORE_NAMESPACE) class EventWaveform(MultiContainerInterface): """ - DEPRECATED. Spike data for spike events detected in raw data + DEPRECATED as of NWB 2.8.0 and PyNWB 3.0.0. + Spike data for spike events detected in raw data stored in this NWBFile, or events detect at acquisition """ @@ -174,10 +175,11 @@ class EventWaveform(MultiContainerInterface): } def __init__(self, **kwargs): - raise ValueError( - "This neurodata type is deprecated. If you are interested in using it, " - "please create an issue on https://github.com/NeurodataWithoutBorders/nwb-schema/issues." - ) + if not self._in_construct_mode: + raise ValueError( + "The EventWaveform neurodata type is deprecated. If you are interested in using it, " + "please create an issue on https://github.com/NeurodataWithoutBorders/nwb-schema/issues." + ) @register_class('Clustering', CORE_NAMESPACE)