Skip to content

Accept zarr v3 arrays in array-valued fields; fix ElectricalSeries len() - #2235

Open
rly wants to merge 2 commits into
devfrom
fix/zarr-v3-iterable-and-len
Open

Accept zarr v3 arrays in array-valued fields; fix ElectricalSeries len()#2235
rly wants to merge 2 commits into
devfrom
fix/zarr-v3-iterable-and-len

Conversation

@rly

@rly rly commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Motivation

Fixes #2234.

Reading NWB files through the zarr backend (hdmf-zarr's zarr v3 migration) passes lazily-read datasets into container constructors as raw zarr.Array objects. A zarr v3 Array implements neither __iter__ nor __len__, which broke pynwb in two places:

  1. Fields declared with docval type=collections.abc.Iterable rejected zarr arrays, because isinstance(zarr_array, Iterable) is False (the ABC only recognizes __iter__; iteration otherwise works via the __getitem__ sequence protocol).
  2. ElectricalSeries.__init__ called len(electrodes.data) directly, raising TypeError: object of type 'Array' has no len().

hdmf already accepts zarr v3 arrays in its own type system (it registers zarr.Array in the array_data docval macro), so both issues are pynwb-side.

Changes

  • Retype array-valued fields from Iterable to ('array_data', 'data'): TimeSeries.control/control_description, ImageSeries.dimension/starting_frame, AbstractFeatureSeries.features/feature_units, and the deprecated Clustering.peak_over_rms, ClusterWaveforms.waveform_mean/waveform_sd (still constructed on read).
  • Leave NWBFile.electrode_groups as Iterable: it holds ElectrodeGroup objects, not array data, and is never backed by a dataset.
  • In ElectricalSeries.__init__, derive the electrode count via get_data_shape(...)[0] instead of len(electrodes.data), matching the pattern already used for data.
  • Add tests/unit/test_array_data_fields.py guarding the retype (array accepted, non-array iterable rejected) and the electrode_groups exclusion.

Behavior change

('array_data', 'data') accepts zarr.Array but does not accept non-array iterables (str, set, range, generators) for these fields, which Iterable previously allowed. For these array-valued fields that is a desirable tightening. Noted in the changelog.

How tested

  • Full unit + integration suite: 795 passed, 8 skipped, 6683 subtests passed.
  • End to end against zarr v3: with hdmf-zarr's zarr v3 migration (no zarr.Array monkey-patches) and this branch, hdmf-zarr's zarr-v2-backward-compat read suite goes from 3 failed + 57 errors to 61 passed.

Checklist

  • Did you update CHANGELOG.md with your changes?
  • Did you add/update tests?

🤖 Generated with Claude Code

rly and others added 2 commits July 29, 2026 18:54
Retype the array-valued fields declared collections.abc.Iterable to the
('array_data', 'data') docval type so array-API arrays such as a zarr v3
Array (which implement neither __iter__ nor __len__) are accepted on read:
TimeSeries.control/control_description, ImageSeries.dimension/starting_frame,
AbstractFeatureSeries.features/feature_units, and the deprecated
Clustering.peak_over_rms and ClusterWaveforms.waveform_mean/waveform_sd.
NWBFile.electrode_groups stays Iterable; it holds ElectrodeGroup objects,
not array data.

Derive the electrode count in ElectricalSeries.__init__ from get_data_shape
instead of len(electrodes.data), which raised TypeError on a zarr v3 Array.

See #2234.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@codecov

codecov Bot commented Jul 30, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.02%. Comparing base (eb08f14) to head (9ccb271).

Additional details and impacted files
@@            Coverage Diff             @@
##              dev    #2235      +/-   ##
==========================================
- Coverage   96.02%   96.02%   -0.01%     
==========================================
  Files          30       30              
  Lines        2997     2995       -2     
  Branches      436      436              
==========================================
- Hits         2878     2876       -2     
  Misses         67       67              
  Partials       52       52              
Flag Coverage Δ
integration 74.55% <66.66%> (-0.02%) ⬇️
unit 86.17% <100.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Reading zarr v3 datasets fails: type=Iterable fields and a direct len() call

1 participant