Require TimeSeries.data and .unit, update ImageSeries, OpticalSeries (#…

…1274)

Use latest nwb schema

CHANGELOG.md

@@ -4,16 +4,23 @@
 
 ### Breaking changes:
 - ``SweepTable`` has been deprecated in favor of the new icephys metadata tables. Use of ``SweepTable``
-  is still possible but no longer recommended. @oruebel  (#1349)
+  is still possible but no longer recommended. @oruebel  (#1349
+- ``TimeSeries.__init__`` now requires the ``data`` argument because the 'data' dataset is required by the schema.
+  If a ``TimeSeries`` is read without a value for ``data``, it will be set to a default value. For most
+  ``TimeSeries``, this is a 1-dimensional empty array with dtype uint8. For ``ImageSeries`` and
+  ``DecompositionSeries``, this is a 3-dimensional empty array with dtype uint8. @rly (#1274)
+- ``TimeSeries.__init__`` now requires the ``unit`` argument because the 'unit' attribute is required by the schema.
+  If a ``TimeSeries`` is read without a value for ``unit``, it will be set to a default value. For most
+  ``TimeSeries``, this is "unknown". For ``IndexSeries``, this is "N/A" according to the NWB 2.4.0 schema. @rly (#1274)
 
 ### New features:
 - Added new intracellular electrophysiology hierarchical table structure from ndx-icephys-meta to NWB core.
   This includes the new types ``TimeSeriesReferenceVectorData``, ``IntracellularRecordingsTable``,
   ``SimultaneousRecordingsTable``, ``SequentialRecordingsTable``, ``RepetitionsTable`` and
   ``ExperimentalConditionsTable`` as well as corresponding updates to ``NWBFile`` to support interaction
    with the new tables. @oruebel  (#1349)
-- Added support for nwb-schema 2.4.0. See [Release Notes](https://nwb-schema.readthedocs.io/en/latest/format_release_notes.html)
-  for more details. @oruebel (#1349)
+- Added support for NWB 2.4.0. See [Release Notes](https://nwb-schema.readthedocs.io/en/latest/format_release_notes.html)
+  for more details. @oruebel, @rly (#1349)
 - Dropped Python 3.6 support, added Python 3.9 support. @rly (#1377)
 - Updated requirements to allow compatibility with HDMF 3 and h5py 3. @rly (#1377)
 
@@ -37,8 +44,12 @@
 - Enforced electrode ID uniqueness during insertion into table. @CodyCBakerPhD (#1344)
 - Fixed integration tests with invalid test data that will be caught by future hdmf validator version.
   @dsleiter, @rly (#1366, #1376)
-- Fixed build warnings in docs @oruebel (#1380)
-- Fix intersphinx links in docs for numpy @oruebel (#1386)
+- Fixed build warnings in docs. @oruebel (#1380)
+- Fix intersphinx links in docs for numpy. @oruebel (#1386)
+- Previously, the ``data`` argument was required in ``OpticalSeries.__init__`` even though ``external_file`` could
+  be provided in place of ``data``. ``OpticalSeries.__init__`` now makes ``data`` optional. However, this has the
+  side effect of moving the position of ``data`` to later in the argument list, which may break code that relies
+  on positional arguments for ``OpticalSeries.__init__``. @rly (#1274)
 
 ## PyNWB 1.5.1 (May 24, 2021)
 

docs/gallery/domain/ophys.py

@@ -152,7 +152,13 @@
 
 data = [0., 1., 2., 3., 4., 5., 6., 7., 8., 9.]
 timestamps = [0.0, 0.1, 0.2, 0.3, 0.4, 0.5, 0.6, 0.7, 0.8, 0.9]
-rrs = fl.create_roi_response_series('my_rrs', data, rt_region, unit='lumens', timestamps=timestamps)
+rrs = fl.create_roi_response_series(
+    name='my_rrs',
+    data=data,
+    rois=rt_region,
+    unit='lumens',
+    timestamps=timestamps
+)
 
 
 ####################

src/pynwb/base.py

@@ -1,11 +1,10 @@
 from warnings import warn
 from collections.abc import Iterable
+import numpy as np
 from typing import NamedTuple
 
 from hdmf.utils import docval, getargs, popargs, call_docval_func, get_docval
 from hdmf.common import DynamicTable, VectorData
-import numpy as np
-
 
 from . import register_class, CORE_NAMESPACE
 from .core import NWBDataInterface, MultiContainerInterface, NWBData
@@ -99,12 +98,15 @@ class TimeSeries(NWBDataInterface):
 
     __time_unit = "seconds"
 
+    # values used when a TimeSeries is read and missing required fields
+    DEFAULT_DATA = np.ndarray(shape=(0, ), dtype=np.uint8)
+    DEFAULT_UNIT = 'unknown'
+
     @docval({'name': 'name', 'type': str, 'doc': 'The name of this TimeSeries dataset'},  # required
             {'name': 'data', 'type': ('array_data', 'data', 'TimeSeries'),
              'doc': ('The data values. The first dimension must be time. '
-                     'Can also store binary data, e.g., image frames'),
-             'default': None},
-            {'name': 'unit', 'type': str, 'doc': 'The base unit of measurement (should be SI unit)', 'default': None},
+                     'Can also store binary data, e.g., image frames')},
+            {'name': 'unit', 'type': str, 'doc': 'The base unit of measurement (should be SI unit)'},
             {'name': 'resolution', 'type': (str, 'float'),
              'doc': 'The smallest meaningful difference (in specified unit) between values in data', 'default': -1.0},
             {'name': 'conversion', 'type': (str, 'float'),

src/pynwb/image.py

@@ -1,7 +1,8 @@
 import warnings
+import numpy as np
 from collections.abc import Iterable
 
-from hdmf.utils import docval, popargs, call_docval_func, get_docval
+from hdmf.utils import docval, getargs, popargs, call_docval_func, get_docval
 
 from . import register_class, CORE_NAMESPACE
 from .base import TimeSeries, Image
@@ -21,18 +22,27 @@ class ImageSeries(TimeSeries):
                      'format',
                      'device')
 
+    # value used when an ImageSeries is read and missing data
+    DEFAULT_DATA = np.ndarray(shape=(0, 0, 0), dtype=np.uint8)
+    # TODO: copy new docs from 2.4 schema
+
     @docval(*get_docval(TimeSeries.__init__, 'name'),  # required
             {'name': 'data', 'type': ('array_data', 'data', TimeSeries), 'shape': ([None] * 3, [None] * 4),
              'doc': ('The data values. Can be 3D or 4D. The first dimension must be time (frame). The second and third '
-                     'dimensions represent x and y. The optional fourth dimension represents z.'),
+                     'dimensions represent x and y. The optional fourth dimension represents z. Either data or '
+                     'external_file must be specified (not None), but not both. If data is not specified, '
+                     'data will be set to an empty 3D array.'),
+             'default': None},
+            {'name': 'unit', 'type': str,
+             'doc': ('The unit of measurement of the image data, e.g., values between 0 and 255. Required when data '
+                     'is specified. If unit (and data) are not specified, then unit will be set to "unknown".'),
              'default': None},
-            *get_docval(TimeSeries.__init__, 'unit'),
             {'name': 'format', 'type': str,
              'doc': 'Format of image. Three types: 1) Image format; tiff, png, jpg, etc. 2) external 3) raw.',
              'default': None},
             {'name': 'external_file', 'type': ('array_data', 'data'),
              'doc': 'Path or URL to one or more external file(s). Field only present if format=external. '
-                    'Either external_file or data must be specified, but not both.', 'default': None},
+                    'Either external_file or data must be specified (not None), but not both.', 'default': None},
             {'name': 'starting_frame', 'type': Iterable,
              'doc': 'Each entry is the frame number in the corresponding external_file variable. '
                     'This serves as an index to what frames each file contains. If external_file is not '
@@ -48,10 +58,22 @@ class ImageSeries(TimeSeries):
     def __init__(self, **kwargs):
         bits_per_pixel, dimension, external_file, starting_frame, format, device = popargs(
             'bits_per_pixel', 'dimension', 'external_file', 'starting_frame', 'format', 'device', kwargs)
-        call_docval_func(super(ImageSeries, self).__init__, kwargs)
-        if external_file is None and self.data is None:
+        name, data, unit = getargs('name', 'data', 'unit', kwargs)
+        if data is not None and unit is None:
+            raise ValueError("Must supply 'unit' argument when supplying 'data' to %s '%s'."
+                             % (self.__class__.__name__, name))
+        if external_file is None and data is None:
             raise ValueError("Must supply either external_file or data to %s '%s'."
-                             % (self.__class__.__name__, self.name))
+                             % (self.__class__.__name__, name))
+
+        # data and unit are required in TimeSeries, but allowed to be None here, so handle this specially
+        if data is None:
+            kwargs['data'] = ImageSeries.DEFAULT_DATA
+        if unit is None:
+            kwargs['unit'] = ImageSeries.DEFAULT_UNIT
+
+        call_docval_func(super(ImageSeries, self).__init__, kwargs)
+
         self.bits_per_pixel = bits_per_pixel
         self.dimension = dimension
         self.external_file = external_file
@@ -76,7 +98,7 @@ def bits_per_pixel(self, val):
 @register_class('IndexSeries', CORE_NAMESPACE)
 class IndexSeries(TimeSeries):
     '''
-    Stores indices to image frames stored in an ImageSeries. The purpose of the ImageIndexSeries is to allow
+    Stores indices to image frames stored in an ImageSeries. The purpose of the IndexSeries is to allow
     a static image stack to be stored somewhere, and the images in the stack to be referenced out-of-order.
     This can be for the display of individual images, or of movie segments (as a movie is simply a series of
     images). The data field stores the index of the frame in the referenced ImageSeries, and the timestamps
@@ -85,10 +107,13 @@ class IndexSeries(TimeSeries):
 
     __nwbfields__ = ('indexed_timeseries',)
 
+    # # value used when an ImageSeries is read and missing data
+    # DEFAULT_UNIT = 'N/A'
+
     @docval(*get_docval(TimeSeries.__init__, 'name'),  # required
             {'name': 'data', 'type': ('array_data', 'data', TimeSeries), 'shape': (None, ),  # required
-             'doc': ('The data values. Must be 1D, where the first dimension must be time (frame)')},
-            *get_docval(TimeSeries.__init__, 'unit'),
+            'doc': ('The data values. Must be 1D, where the first dimension must be time (frame)')},
+            *get_docval(TimeSeries.__init__, 'unit'),  # required
             {'name': 'indexed_timeseries', 'type': TimeSeries,  # required
              'doc': 'HDF5 link to TimeSeries containing images that are indexed.'},
             *get_docval(TimeSeries.__init__, 'resolution', 'conversion', 'timestamps', 'starting_time', 'rate',
@@ -111,12 +136,11 @@ class ImageMaskSeries(ImageSeries):
     __nwbfields__ = ('masked_imageseries',)
 
     @docval(*get_docval(ImageSeries.__init__, 'name'),  # required
-            *get_docval(ImageSeries.__init__, 'data', 'unit'),
             {'name': 'masked_imageseries', 'type': ImageSeries,  # required
              'doc': 'Link to ImageSeries that mask is applied to.'},
-            *get_docval(ImageSeries.__init__, 'format', 'external_file', 'starting_frame', 'bits_per_pixel',
-                        'dimension', 'resolution', 'conversion', 'timestamps', 'starting_time', 'rate', 'comments',
-                        'description', 'control', 'control_description'),
+            *get_docval(ImageSeries.__init__, 'data', 'unit', 'format', 'external_file', 'starting_frame',
+                        'bits_per_pixel', 'dimension', 'resolution', 'conversion', 'timestamps', 'starting_time',
+                        'rate', 'comments', 'description', 'control', 'control_description'),
             {'name': 'device', 'type': Device,
              'doc': ('Device used to capture the mask data. This field will likely not be needed. '
                      'The device used to capture the masked ImageSeries data should be stored in the ImageSeries.'),
@@ -141,19 +165,20 @@ class OpticalSeries(ImageSeries):
                      'field_of_view',
                      'orientation')
 
-    @docval(*get_docval(ImageSeries.__init__, 'name'),
-            {'name': 'data', 'type': ('array_data', 'data'), 'shape': ([None] * 3, [None, None, None, 3]),
-             'doc': ('Images presented to subject, either grayscale or RGB. May be 3D or 4D. The first dimension must '
-                     'be time (frame). The second and third dimensions represent x and y. The optional fourth '
-                     'dimension must be length 3 and represents the RGB value for color images.')},
-            *get_docval(ImageSeries.__init__, 'unit', 'format'),
+    @docval(*get_docval(ImageSeries.__init__, 'name'),  # required
             {'name': 'distance', 'type': 'float', 'doc': 'Distance from camera/monitor to target/eye.'},  # required
             {'name': 'field_of_view', 'type': ('array_data', 'data', 'TimeSeries'), 'shape': ((2, ), (3, )),  # required
              'doc': 'Width, height and depth of image, or imaged area (meters).'},
             {'name': 'orientation', 'type': str,  # required
              'doc': 'Description of image relative to some reference frame (e.g., which way is up). '
                     'Must also specify frame of reference.'},
-            *get_docval(ImageSeries.__init__, 'external_file', 'starting_frame', 'bits_per_pixel',
+            {'name': 'data', 'type': ('array_data', 'data'), 'shape': ([None] * 3, [None, None, None, 3]),
+             'doc': ('Images presented to subject, either grayscale or RGB. May be 3D or 4D. The first dimension must '
+                     'be time (frame). The second and third dimensions represent x and y. The optional fourth '
+                     'dimension must be length 3 and represents the RGB value for color images. Either data or '
+                     'external_file must be specified, but not both.'),
+             'default': None},
+            *get_docval(ImageSeries.__init__, 'unit', 'format', 'external_file', 'starting_frame', 'bits_per_pixel',
                         'dimension', 'resolution', 'conversion', 'timestamps', 'starting_time', 'rate', 'comments',
                         'description', 'control', 'control_description', 'device'))
     def __init__(self, **kwargs):

src/pynwb/io/base.py

@@ -72,10 +72,47 @@ def timestamps_carg(self, builder, manager):
             #
             # NOTE: it is not available when data is externally linked
             # and we haven't explicitly read that file
-            if tstamps_builder.builder.parent is not None:
-                target = tstamps_builder.builder
+            target = tstamps_builder.builder
+            if target.parent is not None:
                 return manager.construct(target.parent)
             else:
-                return tstamps_builder.builder.data
+                return target.data
         else:
             return tstamps_builder.data
+
+    @NWBContainerMapper.constructor_arg("data")
+    def data_carg(self, builder, manager):
+        # handle case where a TimeSeries is read and missing data
+        timeseries_cls = manager.get_cls(builder)
+        data_builder = builder.get('data')
+        if data_builder is None:
+            return timeseries_cls.DEFAULT_DATA
+        if isinstance(data_builder, LinkBuilder):
+            # NOTE: parent is not available when data is externally linked
+            # and we haven't explicitly read that file
+            target = data_builder.builder
+            if target.parent is not None:
+                return manager.construct(target.parent)
+            else:
+                return target.data
+        return data_builder.data
+
+    @NWBContainerMapper.constructor_arg("unit")
+    def unit_carg(self, builder, manager):
+        # handle case where a TimeSeries is read and missing unit
+        timeseries_cls = manager.get_cls(builder)
+        data_builder = builder.get('data')
+        if data_builder is None:
+            return timeseries_cls.DEFAULT_UNIT
+        if isinstance(data_builder, LinkBuilder):
+            # NOTE: parent is not available when data is externally linked
+            # and we haven't explicitly read that file
+            target = data_builder.builder
+            if target.parent is not None:
+                data_builder = manager.construct(target.parent)
+            else:
+                data_builder = target
+        unit_value = data_builder.attributes.get('unit')
+        if unit_value is None:
+            return timeseries_cls.DEFAULT_UNIT
+        return unit_value

src/pynwb/io/image.py

@@ -1,5 +1,4 @@
 from .. import register_map
-
 from ..image import ImageSeries
 from .base import TimeSeriesMap
 
@@ -8,6 +7,6 @@
 class ImageSeriesMap(TimeSeriesMap):
 
     def __init__(self, spec):
-        super(ImageSeriesMap, self).__init__(spec)
+        super().__init__(spec)
         external_file_spec = self.spec.get_dataset('external_file')
         self.map_spec('starting_frame', external_file_spec.get_attribute('starting_frame'))

src/pynwb/misc.py

@@ -12,19 +12,16 @@
 
 @register_class('AnnotationSeries', CORE_NAMESPACE)
 class AnnotationSeries(TimeSeries):
-    """
-    Stores text-based records about the experiment. To use the
-    AnnotationSeries, add records individually through
-    add_annotation() and then call finalize(). Alternatively, if
-    all annotations are already stored in a list, use set_data()
-    and set_timestamps()
+    """Stores text-based records about the experiment.
+    To use the AnnotationSeries, add records individually through add_annotation(). Alternatively, if all annotations
+    are already stored in a list or numpy array, set the data and timestamps in the constructor.
     """
 
     __nwbfields__ = ()
 
     @docval(*get_docval(TimeSeries.__init__, 'name'),  # required
             {'name': 'data', 'type': ('array_data', 'data', TimeSeries), 'shape': (None,),
-             'doc': 'The data values over time. Must be 1D.',
+             'doc': 'The annotations over time. Must be 1D.',
              'default': list()},
             *get_docval(TimeSeries.__init__, 'timestamps', 'comments', 'description'))
     def __init__(self, **kwargs):
@@ -34,9 +31,7 @@ def __init__(self, **kwargs):
     @docval({'name': 'time', 'type': 'float', 'doc': 'The time for the annotation'},
             {'name': 'annotation', 'type': str, 'doc': 'the annotation'})
     def add_annotation(self, **kwargs):
-        '''
-        Add an annotation
-        '''
+        """Add an annotation."""
         time, annotation = getargs('time', 'annotation', kwargs)
         self.fields['timestamps'].append(time)
         self.fields['data'].append(annotation)
@@ -255,6 +250,9 @@ class DecompositionSeries(TimeSeries):
                      {'name': 'bands',
                       'doc': 'the bands that the signal is decomposed into', 'child': True})
 
+    # value used when a DecompositionSeries is read and missing data
+    DEFAULT_DATA = np.ndarray(shape=(0, 0, 0), dtype=np.uint8)
+
     @docval(*get_docval(TimeSeries.__init__, 'name'),  # required
             {'name': 'data', 'type': ('array_data', 'data', TimeSeries),  # required
              'doc': ('The data values. Must be 3D, where the first dimension must be time, the second dimension must '