Support more stimulus types (#1842)

NeurodataWithoutBorders · Feb 6, 2024 · 5c760da · 5c760da
1 parent 40e760c
commit 5c760da
Show file tree

Hide file tree

Showing 7 changed files with 75 additions and 11 deletions.
diff --git 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)

diff --git 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
@@ -177,7 +177,7 @@ class NWBFile(MultiContainerInterface, HERDManager):
         {
             'attr': 'stimulus',
             'add': '_add_stimulus_internal',
-            'type': TimeSeries,
+            'type': (NWBDataInterface, DynamicTable),
             '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,29 @@ 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': '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):
-        timeseries = popargs('timeseries', kwargs)
-        self._add_stimulus_internal(timeseries)
+        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' 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 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)
         use_sweep_table = popargs('use_sweep_table', kwargs)
         if use_sweep_table:
-            self._update_sweep_table(timeseries)
+            self._update_sweep_table(stimulus)
 
     @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
@@ -31,7 +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'))
+        # 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
diff --git 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",  # TODO cannot delete this file on windows for some reason
+        "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

diff --git 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,43 @@ 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',
+            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]))
+10 −0		core/nwb.file.yaml
+3 −3		core/nwb.retinotopy.yaml
+3 −1		docs/format/source/format_release_notes.rst