diff --git a/CHANGELOG.md b/CHANGELOG.md index 3b08ecbfe..eaaceb751 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,13 @@ # PyNWB Changelog +## PyNWB 4.2.0 (Upcoming) + +### Changed +- Retyped array-valued fields that were declared `collections.abc.Iterable` to the `('array_data', 'data')` docval type: `TimeSeries.control` and `control_description`, `ImageSeries.dimension` and `starting_frame`, `AbstractFeatureSeries.features` and `feature_units`, and the deprecated `Clustering.peak_over_rms`, `ClusterWaveforms.waveform_mean`, and `ClusterWaveforms.waveform_sd`. A zarr v3 `Array` implements neither `__iter__` nor `__len__`, so `isinstance(zarr_array, Iterable)` is `False` and these fields rejected zarr-backed data on read; the `array_data` macro includes `zarr.Array`, so they accept it. The `('array_data', 'data')` type does not accept non-array iterables (`str`, `set`, `range`, generators) for these fields. `NWBFile.electrode_groups` is unchanged: it holds `ElectrodeGroup` objects, not array data. @rly [#2235](https://github.com/NeurodataWithoutBorders/pynwb/pull/2235) + +### Fixed +- Fixed `ElectricalSeries.__init__` raising `TypeError: object of type 'Array' has no len()` when the `electrodes` region was backed by a zarr v3 `Array`. The electrode count used for the data-orientation check is derived via `get_data_shape` instead of `len()`. @rly [#2235](https://github.com/NeurodataWithoutBorders/pynwb/pull/2235) + ## PyNWB 4.1.0 (July 23, 2026) ### Changed diff --git a/src/pynwb/base.py b/src/pynwb/base.py index f1a57b7c0..0b5d5e776 100644 --- a/src/pynwb/base.py +++ b/src/pynwb/base.py @@ -1,5 +1,4 @@ from warnings import warn -from collections.abc import Iterable from abc import ABC from typing import NamedTuple @@ -159,9 +158,9 @@ class TimeSeries(NWBDataInterface): 'default': 'no comments'}, {'name': 'description', 'type': str, 'doc': 'Description of this TimeSeries dataset', 'default': 'no description'}, - {'name': 'control', 'type': Iterable, 'doc': 'Numerical labels that apply to each element in data', - 'default': None}, - {'name': 'control_description', 'type': Iterable, 'doc': 'Description of each control value', + {'name': 'control', 'type': ('array_data', 'data'), + 'doc': 'Numerical labels that apply to each element in data', 'default': None}, + {'name': 'control_description', 'type': ('array_data', 'data'), 'doc': 'Description of each control value', 'default': None}, {'name': 'continuity', 'type': str, 'default': None, 'enum': ["continuous", "instantaneous", "step"], 'doc': 'Optionally describe the continuity of the data. Can be "continuous", "instantaneous", or ' diff --git a/src/pynwb/ecephys.py b/src/pynwb/ecephys.py index 2fda9ae58..ab3354ae8 100644 --- a/src/pynwb/ecephys.py +++ b/src/pynwb/ecephys.py @@ -1,6 +1,5 @@ import warnings import numpy as np -from collections.abc import Iterable from hdmf.common import DynamicTableRegion, DynamicTable from hdmf.data_utils import assertEqualShape @@ -146,12 +145,15 @@ def __init__(self, **kwargs): args_to_set = popargs_to_dict(('electrodes', 'channel_conversion', 'filtering'), kwargs) data_shape = get_data_shape(kwargs['data'], strict_no_data_load=True) + electrodes_shape = get_data_shape(args_to_set['electrodes'].data, strict_no_data_load=True) + n_electrodes = electrodes_shape[0] if electrodes_shape is not None else None if ( data_shape is not None + and n_electrodes is not None and len(data_shape) == 2 - and data_shape[1] != len(args_to_set['electrodes'].data) + and data_shape[1] != n_electrodes ): - if data_shape[0] == len(args_to_set['electrodes'].data): + if data_shape[0] == n_electrodes: warnings.warn("%s '%s': The second dimension of data does not match the length of electrodes, " "but instead the first does. Data is oriented incorrectly and should be transposed." % (self.__class__.__name__, kwargs["name"])) @@ -300,7 +302,7 @@ class Clustering(NWBDataInterface): 'doc': 'Description of clusters or clustering, (e.g. cluster 0 is noise, ' 'clusters curated using Klusters, etc).'}, {'name': 'num', 'type': ('array_data', 'data'), 'doc': 'Cluster number of each event.', 'shape': (None, )}, - {'name': 'peak_over_rms', 'type': Iterable, 'shape': (None, ), + {'name': 'peak_over_rms', 'type': ('array_data', 'data'), 'shape': (None, ), 'doc': 'Maximum ratio of waveform peak to RMS on any channel in the cluster' '(provides a basic clustering metric).'}, {'name': 'times', 'type': ('array_data', 'data'), 'doc': 'Times of clustered events, in seconds.', @@ -337,9 +339,9 @@ class ClusterWaveforms(NWBDataInterface): 'doc': 'the clustered spike data used as input for computing waveforms'}, {'name': 'waveform_filtering', 'type': str, 'doc': 'filter applied to data before calculating mean and standard deviation'}, - {'name': 'waveform_mean', 'type': Iterable, 'shape': (None, None), + {'name': 'waveform_mean', 'type': ('array_data', 'data'), 'shape': (None, None), 'doc': 'the mean waveform for each cluster'}, - {'name': 'waveform_sd', 'type': Iterable, 'shape': (None, None), + {'name': 'waveform_sd', 'type': ('array_data', 'data'), 'shape': (None, None), 'doc': 'the standard deviations of waveforms for each cluster'}, {'name': 'name', 'type': str, 'doc': 'the name of this container', 'default': 'ClusterWaveforms'}) def __init__(self, **kwargs): diff --git a/src/pynwb/image.py b/src/pynwb/image.py index 1ffad3871..c3d24c629 100644 --- a/src/pynwb/image.py +++ b/src/pynwb/image.py @@ -22,7 +22,6 @@ """ import warnings -from collections.abc import Iterable import numpy as np @@ -86,7 +85,7 @@ class ImageSeries(TimeSeries): {'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 (not None), but not both.', 'default': None}, - {'name': 'starting_frame', 'type': Iterable, + {'name': 'starting_frame', 'type': ('array_data', 'data'), 'doc': 'Each entry is a frame number that corresponds to the first frame of each file ' 'listed in external_file within the full ImageSeries.', 'default': None}, {'name': 'num_samples', 'type': (int, np.unsignedinteger), @@ -97,7 +96,7 @@ class ImageSeries(TimeSeries): 'default': None}, {'name': 'bits_per_pixel', 'type': int, 'doc': 'DEPRECATED: Number of bits per image pixel', 'default': None}, - {'name': 'dimension', 'type': Iterable, + {'name': 'dimension', 'type': ('array_data', 'data'), 'doc': 'Number of pixels on x, y, (and z) axes.', 'default': None}, *get_docval(TimeSeries.__init__, 'resolution', 'conversion', 'timestamps', 'starting_time', 'rate', 'comments', 'description', 'control', 'control_description', 'offset'), diff --git a/src/pynwb/misc.py b/src/pynwb/misc.py index a0549b914..8fd4d0294 100644 --- a/src/pynwb/misc.py +++ b/src/pynwb/misc.py @@ -1,5 +1,4 @@ import warnings -from collections.abc import Iterable from bisect import bisect_left, bisect_right import numpy as np @@ -72,9 +71,9 @@ class AbstractFeatureSeries(TimeSeries): 'features') @docval(*get_docval(TimeSeries.__init__, 'name'), # required - {'name': 'feature_units', 'type': Iterable, 'shape': (None, ), # required + {'name': 'feature_units', 'type': ('array_data', 'data'), 'shape': (None, ), # required 'doc': 'The unit of each feature'}, - {'name': 'features', 'type': Iterable, 'shape': (None, ), # required + {'name': 'features', 'type': ('array_data', 'data'), 'shape': (None, ), # required 'doc': 'Description of each feature'}, {'name': 'data', 'type': ('array_data', 'data', TimeSeries), 'shape': ((None,), (None, None)), 'doc': ('The data values. May be 1D or 2D. The first dimension must be time. The optional second ' diff --git a/tests/unit/test_array_data_fields.py b/tests/unit/test_array_data_fields.py new file mode 100644 index 000000000..73a75fa47 --- /dev/null +++ b/tests/unit/test_array_data_fields.py @@ -0,0 +1,52 @@ +"""Guard the docval types of array-valued fields that accept array-API arrays. + +zarr v3 ``Array`` exposes ``shape`` and ``__getitem__`` but implements neither +``__iter__`` nor ``__len__``, so it does not satisfy +``isinstance(x, collections.abc.Iterable)``. Fields that hold array data therefore +use the ``('array_data', 'data')`` docval type, whose ``array_data`` macro includes +``zarr.Array``. That type accepts array-like values and rejects non-array iterables +such as ``set``. ``NWBFile.electrode_groups`` holds ``ElectrodeGroup`` objects and is +never backed by a dataset, so it uses ``Iterable`` and accepts any iterable. + +See https://github.com/NeurodataWithoutBorders/pynwb/issues/2234. +""" +import numpy as np +from hdmf.utils import check_type, get_docval + +from pynwb.base import TimeSeries +from pynwb.image import ImageSeries +from pynwb.misc import AbstractFeatureSeries +from pynwb.ecephys import Clustering, ClusterWaveforms +from pynwb.file import NWBFile +from pynwb.testing import TestCase + + +def _field_type(func, name): + (spec,) = [d for d in get_docval(func) if d['name'] == name] + return spec['type'] + + +class TestArrayDataFieldTypes(TestCase): + + def test_array_valued_fields_accept_array_data_reject_non_array_iterable(self): + cases = [ + (TimeSeries.__init__, 'control'), + (TimeSeries.__init__, 'control_description'), + (ImageSeries.__init__, 'dimension'), + (ImageSeries.__init__, 'starting_frame'), + (AbstractFeatureSeries.__init__, 'features'), + (AbstractFeatureSeries.__init__, 'feature_units'), + (Clustering.__init__, 'peak_over_rms'), + (ClusterWaveforms.__init__, 'waveform_mean'), + (ClusterWaveforms.__init__, 'waveform_sd'), + ] + for func, name in cases: + with self.subTest(field=name): + field_type = _field_type(func, name) + self.assertTrue(check_type(np.array([0, 1, 2]), field_type)) + self.assertTrue(check_type([0, 1, 2], field_type)) + self.assertFalse(check_type({0, 1, 2}, field_type)) + + def test_electrode_groups_holds_objects_accepts_any_iterable(self): + field_type = _field_type(NWBFile.__init__, 'electrode_groups') + self.assertTrue(check_type({0, 1, 2}, field_type))