From 6b2668d173d0515a622d4c8a66569ba565149528 Mon Sep 17 00:00:00 2001 From: rly Date: Sun, 4 Feb 2024 13:39:48 -0800 Subject: [PATCH 01/10] Stash changes --- src/pynwb/file.py | 23 ++++++++++++++++------- src/pynwb/io/file.py | 2 ++ tests/unit/test_file.py | 10 ++++++++++ 3 files changed, 28 insertions(+), 7 deletions(-) diff --git a/src/pynwb/file.py b/src/pynwb/file.py index b473e571a..7af0588e4 100644 --- a/src/pynwb/file.py +++ b/src/pynwb/file.py @@ -177,7 +177,7 @@ class NWBFile(MultiContainerInterface, HERDManager): { 'attr': 'stimulus', 'add': '_add_stimulus_internal', - 'type': TimeSeries, + 'type': (TimeSeries, DynamicTable, NWBDataInterface), 'get': 'get_stimulus' }, { @@ -356,7 +356,8 @@ class NWBFile(MultiContainerInterface, HERDManager): {'name': 'analysis', 'type': (list, tuple), 'doc': 'result of analysis', 'default': None}, {'name': 'stimulus', 'type': (list, tuple), - 'doc': 'Stimulus TimeSeries objects belonging to this NWBFile', 'default': None}, + 'doc': 'Stimulus TimeSeries, DynamicTable, or NWBDataInterface objects belonging to this NWBFile', + 'default': None}, {'name': 'stimulus_template', 'type': (list, tuple), 'doc': 'Stimulus template TimeSeries objects belonging to this NWBFile', 'default': None}, {'name': 'epochs', 'type': TimeIntervals, @@ -856,14 +857,22 @@ def add_acquisition(self, **kwargs): if use_sweep_table: self._update_sweep_table(nwbdata) - @docval({'name': 'timeseries', 'type': TimeSeries}, - {'name': 'use_sweep_table', 'type': bool, 'default': False, 'doc': 'Use the deprecated SweepTable'}) + @docval({'name': 'nwbdata', 'type': (TimeSeries, DynamicTable, NWBDataInterface), 'default': None, + 'doc': 'The stimulus presentation data to add to this NWBFile.'}, + {'name': 'use_sweep_table', 'type': bool, 'default': False, 'doc': 'Use the deprecated SweepTable'}, + {'name': 'timeseries', 'type': TimeSeries, 'default': None, + 'doc': 'The "timeseries" keyword argument is deprecated. Use the "nwbdata" argument instead.'},) def add_stimulus(self, **kwargs): - timeseries = popargs('timeseries', kwargs) - self._add_stimulus_internal(timeseries) + nwbdata, timeseries = popargs('nwbdata', 'timeseries', kwargs) + if timeseries is not None: + warn("The 'timeseries' keyword argument is deprecated. Use the 'nwbdata' argument instead.", + DeprecationWarning) + if nwbdata is not None: + nwbdata = timeseries + self._add_stimulus_internal(nwbdata) use_sweep_table = popargs('use_sweep_table', kwargs) if use_sweep_table: - self._update_sweep_table(timeseries) + self._update_sweep_table(nwbdata) @docval({'name': 'timeseries', 'type': (TimeSeries, Images)}, {'name': 'use_sweep_table', 'type': bool, 'default': False, 'doc': 'Use the deprecated SweepTable'}) diff --git a/src/pynwb/io/file.py b/src/pynwb/io/file.py index ccbfb8e47..b5e737532 100644 --- a/src/pynwb/io/file.py +++ b/src/pynwb/io/file.py @@ -32,6 +32,8 @@ def __init__(self, spec): self.unmap(stimulus_spec.get_group('presentation')) self.unmap(stimulus_spec.get_group('templates')) self.map_spec('stimulus', stimulus_spec.get_group('presentation').get_neurodata_type('TimeSeries')) + self.map_spec('stimulus', stimulus_spec.get_group('presentation').get_neurodata_type('DynamicTable')) + self.map_spec('stimulus', stimulus_spec.get_group('presentation').get_neurodata_type('NWBDataInterface')) self.map_spec('stimulus_template', stimulus_spec.get_group('templates').get_neurodata_type('TimeSeries')) self.map_spec('stimulus_template', stimulus_spec.get_group('templates').get_neurodata_type('Images')) diff --git a/tests/unit/test_file.py b/tests/unit/test_file.py index 756009ff3..e06885fcf 100644 --- a/tests/unit/test_file.py +++ b/tests/unit/test_file.py @@ -3,6 +3,7 @@ from datetime import datetime, timedelta from dateutil.tz import tzlocal, tzutc +from hdmf.common import DynamicTable from pynwb import NWBFile, TimeSeries, NWBHDF5IO from pynwb.base import Image, Images @@ -149,6 +150,15 @@ def test_add_stimulus(self): 'grams', timestamps=[0.0, 0.1, 0.2, 0.3, 0.4, 0.5])) self.assertEqual(len(self.nwbfile.stimulus), 1) + def test_add_stimulus_dynamic_table(self): + dt = DynamicTable( + name='test_dynamic_table', + description='a test dynamic table', + ) + self.nwbfile.add_stimulus(dt) + self.assertEqual(len(self.nwbfile.stimulus), 1) + self.assertIs(self.nwbfile.stimulus['test_dynamic_table'], dt) + def test_add_stimulus_template(self): self.nwbfile.add_stimulus_template(TimeSeries('test_ts', [0, 1, 2, 3, 4, 5], 'grams', timestamps=[0.0, 0.1, 0.2, 0.3, 0.4, 0.5])) From 8d85f060267873fdfe54979c2e910a0131accc84 Mon Sep 17 00:00:00 2001 From: rly Date: Sun, 4 Feb 2024 13:43:28 -0800 Subject: [PATCH 02/10] Update schema --- 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 b4f8838cb..c7de87a69 160000 --- a/src/pynwb/nwb-schema +++ b/src/pynwb/nwb-schema @@ -1 +1 @@ -Subproject commit b4f8838cbfbb7f8a117bd7e0aad19133d26868b4 +Subproject commit c7de87a6963f7b6b1d172fd6fc23db58652a6f2e From 921d44d8714dd6ca4ff217d651db1a5f14241241 Mon Sep 17 00:00:00 2001 From: rly Date: Mon, 5 Feb 2024 10:51:12 -0800 Subject: [PATCH 03/10] Fix mapper --- src/pynwb/file.py | 2 +- src/pynwb/io/file.py | 6 ++++-- src/pynwb/nwb-schema | 2 +- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/pynwb/file.py b/src/pynwb/file.py index 7af0588e4..3389946ae 100644 --- a/src/pynwb/file.py +++ b/src/pynwb/file.py @@ -177,7 +177,7 @@ class NWBFile(MultiContainerInterface, HERDManager): { 'attr': 'stimulus', 'add': '_add_stimulus_internal', - 'type': (TimeSeries, DynamicTable, NWBDataInterface), + 'type': (NWBDataInterface, DynamicTable), 'get': 'get_stimulus' }, { diff --git a/src/pynwb/io/file.py b/src/pynwb/io/file.py index b5e737532..1908c6b31 100644 --- a/src/pynwb/io/file.py +++ b/src/pynwb/io/file.py @@ -31,9 +31,11 @@ def __init__(self, spec): self.unmap(stimulus_spec) self.unmap(stimulus_spec.get_group('presentation')) self.unmap(stimulus_spec.get_group('templates')) - self.map_spec('stimulus', stimulus_spec.get_group('presentation').get_neurodata_type('TimeSeries')) - self.map_spec('stimulus', stimulus_spec.get_group('presentation').get_neurodata_type('DynamicTable')) + # map "stimulus" to NWBDataInterface and DynamicTable and unmap the spec for TimeSeries because it is + # included in the mapping to NWBDataInterface + self.unmap(stimulus_spec.get_group('presentation').get_neurodata_type('TimeSeries')) self.map_spec('stimulus', stimulus_spec.get_group('presentation').get_neurodata_type('NWBDataInterface')) + self.map_spec('stimulus', stimulus_spec.get_group('presentation').get_neurodata_type('DynamicTable')) self.map_spec('stimulus_template', stimulus_spec.get_group('templates').get_neurodata_type('TimeSeries')) self.map_spec('stimulus_template', stimulus_spec.get_group('templates').get_neurodata_type('Images')) diff --git a/src/pynwb/nwb-schema b/src/pynwb/nwb-schema index c7de87a69..738ba3e81 160000 --- a/src/pynwb/nwb-schema +++ b/src/pynwb/nwb-schema @@ -1 +1 @@ -Subproject commit c7de87a6963f7b6b1d172fd6fc23db58652a6f2e +Subproject commit 738ba3e8139118413c683051ba114d8f5fea92cf From a93443d02cd7878bf6ba4536623abf63142c1824 Mon Sep 17 00:00:00 2001 From: rly Date: Mon, 5 Feb 2024 11:04:24 -0800 Subject: [PATCH 04/10] update schema --- 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 738ba3e81..82da898b3 160000 --- a/src/pynwb/nwb-schema +++ b/src/pynwb/nwb-schema @@ -1 +1 @@ -Subproject commit 738ba3e8139118413c683051ba114d8f5fea92cf +Subproject commit 82da898b331dbccd97e26aedc92c9f9ead330e9d From 90305b5725a384d9747be0476e60ce97c7dea951 Mon Sep 17 00:00:00 2001 From: rly Date: Mon, 5 Feb 2024 12:54:15 -0800 Subject: [PATCH 05/10] Fix add_stimulus --- docs/gallery/domain/images.py | 2 +- src/pynwb/file.py | 19 ++++++++++++------- 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/docs/gallery/domain/images.py b/docs/gallery/domain/images.py index 8b59b75f0..fafc1849f 100644 --- a/docs/gallery/domain/images.py +++ b/docs/gallery/domain/images.py @@ -105,7 +105,7 @@ description="The images presented to the subject as stimuli", ) -nwbfile.add_stimulus(timeseries=optical_series) +nwbfile.add_stimulus(stimulus=optical_series) #################### # ImageSeries: Storing series of images as acquisition diff --git a/src/pynwb/file.py b/src/pynwb/file.py index 3389946ae..e3f412dd5 100644 --- a/src/pynwb/file.py +++ b/src/pynwb/file.py @@ -857,22 +857,27 @@ def add_acquisition(self, **kwargs): if use_sweep_table: self._update_sweep_table(nwbdata) - @docval({'name': 'nwbdata', 'type': (TimeSeries, DynamicTable, NWBDataInterface), 'default': None, + @docval({'name': 'stimulus', 'type': (TimeSeries, DynamicTable, NWBDataInterface), 'default': None, 'doc': 'The stimulus presentation data to add to this NWBFile.'}, {'name': 'use_sweep_table', 'type': bool, 'default': False, 'doc': 'Use the deprecated SweepTable'}, {'name': 'timeseries', 'type': TimeSeries, 'default': None, 'doc': 'The "timeseries" keyword argument is deprecated. Use the "nwbdata" argument instead.'},) def add_stimulus(self, **kwargs): - nwbdata, timeseries = popargs('nwbdata', 'timeseries', kwargs) + stimulus, timeseries = popargs('stimulus', 'timeseries', kwargs) + if stimulus is None and timeseries is None: + raise ValueError( + "The 'stimulus' keyword argument is required. The 'timeseries' keyword argument can be " + "provided for backwards compatibility but is deprecated in favor of 'stimulus'." + ) if timeseries is not None: - warn("The 'timeseries' keyword argument is deprecated. Use the 'nwbdata' argument instead.", + warn("The 'timeseries' keyword argument is deprecated. Use the 'stimulus' argument instead.", DeprecationWarning) - if nwbdata is not None: - nwbdata = timeseries - self._add_stimulus_internal(nwbdata) + if stimulus is None: + stimulus = timeseries + self._add_stimulus_internal(stimulus) use_sweep_table = popargs('use_sweep_table', kwargs) if use_sweep_table: - self._update_sweep_table(nwbdata) + self._update_sweep_table(stimulus) @docval({'name': 'timeseries', 'type': (TimeSeries, Images)}, {'name': 'use_sweep_table', 'type': bool, 'default': False, 'doc': 'Use the deprecated SweepTable'}) From 18c854058381f8fbc6603f9628c5147d05ab13bb Mon Sep 17 00:00:00 2001 From: rly Date: Mon, 5 Feb 2024 12:54:24 -0800 Subject: [PATCH 06/10] Clean up after tests --- test.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/test.py b/test.py index 16191ae3f..c251fd838 100755 --- a/test.py +++ b/test.py @@ -7,6 +7,7 @@ import logging import os.path import os +import shutil from subprocess import run, PIPE, STDOUT import sys import traceback @@ -275,6 +276,9 @@ def clean_up_tests(): "processed_data.nwb", "raw_data.nwb", "scratch_analysis.nwb", + "sub-P11HMH_ses-20061101_ecephys+image.nwb", + "test_edit.nwb", + "test_edit2.nwb", "test_cortical_surface.nwb", "test_icephys_file.nwb", "test_multicontainerinterface.extensions.yaml", @@ -286,6 +290,8 @@ def clean_up_tests(): if os.path.exists(name): os.remove(name) + shutil.rmtree("zarr_tutorial.nwb.zarr") + def main(): # setup and parse arguments From 650a9f97cc39ee35462ca32a1498fffc51707551 Mon Sep 17 00:00:00 2001 From: rly Date: Mon, 5 Feb 2024 12:59:44 -0800 Subject: [PATCH 07/10] Add tests --- src/pynwb/file.py | 8 +++++--- tests/unit/test_file.py | 28 ++++++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 3 deletions(-) diff --git a/src/pynwb/file.py b/src/pynwb/file.py index e3f412dd5..90be96a62 100644 --- a/src/pynwb/file.py +++ b/src/pynwb/file.py @@ -867,11 +867,13 @@ def add_stimulus(self, **kwargs): if stimulus is None and timeseries is None: raise ValueError( "The 'stimulus' keyword argument is required. The 'timeseries' keyword argument can be " - "provided for backwards compatibility but is deprecated in favor of 'stimulus'." + "provided for backwards compatibility but is deprecated in favor of 'stimulus' and will be " + "removed in PyNWB 3.0." ) + # TODO remove this support in PyNWB 3.0 if timeseries is not None: - warn("The 'timeseries' keyword argument is deprecated. Use the 'stimulus' argument instead.", - DeprecationWarning) + warn("The 'timeseries' keyword argument is deprecated and will be removed in PyNWB 3.0. " + "Use the 'stimulus' argument instead.", DeprecationWarning) if stimulus is None: stimulus = timeseries self._add_stimulus_internal(stimulus) diff --git a/tests/unit/test_file.py b/tests/unit/test_file.py index e06885fcf..0fa065e1a 100644 --- a/tests/unit/test_file.py +++ b/tests/unit/test_file.py @@ -150,6 +150,34 @@ def test_add_stimulus(self): 'grams', timestamps=[0.0, 0.1, 0.2, 0.3, 0.4, 0.5])) self.assertEqual(len(self.nwbfile.stimulus), 1) + def test_add_stimulus_timeseries_arg(self): + """Test nwbfile.add_stimulus using the deprecated 'timeseries' keyword argument""" + msg = ( + "The 'timeseries' keyword argument is deprecated and will be removed in PyNWB 3.0. " + "Use the 'stimulus' argument instead." + ) + with self.assertWarnsWith(DeprecationWarning, msg): + self.nwbfile.add_stimulus( + timeseries=TimeSeries( + name='test_ts', + data=[0, 1, 2, 3, 4, 5], + unit='grams', + timestamps=[0.0, 0.1, 0.2, 0.3, 0.4, 0.5] + ) + ) + self.assertEqual(len(self.nwbfile.stimulus), 1) + + def test_add_stimulus_no_stimulus_arg(self): + """Test nwbfile.add_stimulus using the deprecated 'timeseries' keyword argument""" + msg = ( + "The 'stimulus' keyword argument is required. The 'timeseries' keyword argument can be " + "provided for backwards compatibility but is deprecated in favor of 'stimulus' and will be " + "removed in PyNWB 3.0." + ) + with self.assertRaisesWith(ValueError, msg): + self.nwbfile.add_stimulus(None) + self.assertEqual(len(self.nwbfile.stimulus), 0) + def test_add_stimulus_dynamic_table(self): dt = DynamicTable( name='test_dynamic_table', From 73071eeaaf8f440208ab5849ba09a8cc2c4028d8 Mon Sep 17 00:00:00 2001 From: rly Date: Mon, 5 Feb 2024 13:43:34 -0800 Subject: [PATCH 08/10] Update test cleanup for windows --- test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test.py b/test.py index c251fd838..8f6a403ee 100755 --- a/test.py +++ b/test.py @@ -276,7 +276,7 @@ def clean_up_tests(): "processed_data.nwb", "raw_data.nwb", "scratch_analysis.nwb", - "sub-P11HMH_ses-20061101_ecephys+image.nwb", + # "sub-P11HMH_ses-20061101_ecephys+image.nwb", # TODO cannot delete this file on windows for some reason "test_edit.nwb", "test_edit2.nwb", "test_cortical_surface.nwb", From 5cbe3f55279f74e12470a37656a451f0234af671 Mon Sep 17 00:00:00 2001 From: rly Date: Mon, 5 Feb 2024 13:47:31 -0800 Subject: [PATCH 09/10] Update changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1180bb644..e2aece13c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,7 +8,7 @@ - Modified `OptogeneticSeries` to allow 2D data, primarily in extensions of `OptogeneticSeries`. @rly [#1812](https://github.com/NeurodataWithoutBorders/pynwb/pull/1812) - Support `stimulus_template` as optional predefined column in `IntracellularStimuliTable`. @stephprince [#1815](https://github.com/NeurodataWithoutBorders/pynwb/pull/1815) - ... - - ... + - Support `NWBDataInterface` and `DynamicTable` in `NWBFile.stimulus`. @rly [#1842](https://github.com/NeurodataWithoutBorders/pynwb/pull/1842) - For `NWBHDF5IO()`, change the default of arg `load_namespaces` from `False` to `True`. @bendichter [#1748](https://github.com/NeurodataWithoutBorders/pynwb/pull/1748) - Add `NWBHDF5IO.can_read()`. @bendichter [#1703](https://github.com/NeurodataWithoutBorders/pynwb/pull/1703) - Add `pynwb.get_nwbfile_version()`. @bendichter [#1703](https://github.com/NeurodataWithoutBorders/pynwb/pull/1703) From 32a61ff61d0e7599735d8393bce062ac7fac8791 Mon Sep 17 00:00:00 2001 From: rly Date: Mon, 5 Feb 2024 14:06:34 -0800 Subject: [PATCH 10/10] Update schema to latest 2.7.0 dev --- 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 82da898b3..10d2fc202 160000 --- a/src/pynwb/nwb-schema +++ b/src/pynwb/nwb-schema @@ -1 +1 @@ -Subproject commit 82da898b331dbccd97e26aedc92c9f9ead330e9d +Subproject commit 10d2fc202151e1136e01f353f9907f4bf974d3ad