Add Zarr v3 reader - #853
Open
ehennestad wants to merge 15 commits into
Open
Conversation
ehennestad
force-pushed
the
zarr-support/5-zarr3-reader
branch
from
July 10, 2026 05:17
a7c904d to
4d725c0
Compare
ehennestad
force-pushed
the
zarr-support/5-zarr3-reader
branch
from
July 19, 2026 15:26
8681c8f to
f7dc222
Compare
ehennestad
force-pushed
the
zarr-support/5-zarr3-reader
branch
from
August 20, 2026 10:25
e9357f3 to
ab149aa
Compare
ehennestad
commented
Aug 22, 2026
Collaborator
Author
There was a problem hiding this comment.
In this file, docstrings are added to the abstract methods of the LazyArray base class.
ehennestad
force-pushed
the
zarr-support/5-zarr3-reader
branch
2 times, most recently
from
August 24, 2026 18:13
c9a2037 to
69719dd
Compare
ehennestad
force-pushed
the
zarr-support/5-zarr3-reader
branch
2 times, most recently
from
August 25, 2026 07:54
9287570 to
6a0a2b2
Compare
ehennestad
force-pushed
the
backend-agnostic-links
branch
from
August 25, 2026 08:03
7e62552 to
c359d06
Compare
ehennestad
force-pushed
the
zarr-support/5-zarr3-reader
branch
3 times, most recently
from
August 25, 2026 09:01
53cbb3f to
ce1040c
Compare
ehennestad
marked this pull request as ready for review
August 25, 2026 09:18
ehennestad
marked this pull request as draft
August 25, 2026 09:18
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #853 +/- ##
==========================================
+ Coverage 95.31% 95.70% +0.38%
==========================================
Files 234 246 +12
Lines 8329 8794 +465
==========================================
+ Hits 7939 8416 +477
+ Misses 390 378 -12 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
ehennestad
marked this pull request as ready for review
August 25, 2026 13:40
ehennestad
force-pushed
the
zarr-support/5-zarr3-reader
branch
from
August 26, 2026 18:07
409a719 to
f7820e6
Compare
Links (zarr_link), object references (attribute form, reference datasets, "object" compound fields) and the root .specloc attribute are now decoded through the hdmf.zarr namespace (hdmf.zarr.Link, hdmf.zarr.Reference, hdmf.zarr.isReferenceArray, hdmf.zarr.File.specLoc) instead of hand-rolled JSON parsing. A shared io.internal.zarr3.decodeObjectReferences converts references to types.untyped.ObjectView and rejects references into other stores explicitly. hdmf-zarr-matlab is added to requirements.txt; ensureAvailable and the test path helpers (getZarr3DependencyPaths, assumeZarr3Support) resolve both packages. The Zarr v3 fixture writes its links/references via hdmf.zarr.File, adds an external link, a reference dataset and a real .specloc key, and tags the compound array with hdmf-zarr's per-field zarr_dtype list; reader tests cover the new cases. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Document the LazyArray interface contract on the base class, following the base/Reader.m precedent in the same package: the meaning of load_h5_style, load_mat_style, refreshSizeInfo and resolveDataType is identical across backends, so Zarr3LazyArray inherits it rather than restating it. HDF5LazyArray's own per-file help still takes precedence for HDF5. Convert the new Zarr v3 files to matnwb's help style: "% " prose rather than MathWorks "% " syntax paragraphs, class and method help at the declaration's indent, "See also:" listing functions. Fix defects found while reviewing: a reference to a Zarr3Writer class that does not exist on this branch (and which MATLAB served as readDatasetValue's help), seven H1 lines that wrapped onto a second line so help and lookfor showed only a fragment, and a "See also:" holding bare URLs instead of function names. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Fix the four Code Analyzer warnings on the Zarr v3 backend: - Zarr3Reader.NodeInfoMap defaulted to a containers.Map, which is a handle, so one map object was shared by every reader instance. Default to [] and build it in ensureMetadataCache. - Drop two stale %#ok<AGROW> suppressions in buildNodeInfo. - Suppress the NASGU false positive on buildNodeInfo's final map insertion, which the caller does see through handle semantics. - Use isscalar instead of a numel comparison in Zarr3LazyArray. Rename `i` to `iField` in getCompoundFieldSemantics, and `k`/`f` to `iField`/`fieldInfo` in getCompoundTypeDescriptor, matching the iterator naming used elsewhere in the backend. Record in mustBeFile's help that a ".zarr" path is validated as a folder, matching the update already made to mustBeNwbFile. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The reference and link value classes the Zarr v3 backend depends on have been merged, so the temporary pin to the extract-reference-link-value- classes branch is no longer needed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Zarr3Reader maintains its own node model (buildNodeInfo, NodeInfoMap), so it is exactly the consumer hdmf-zarr-matlab intends to use the store-free conventions layer and skip File. It held a File only to open the root and to read the ".specloc" attribute, while opening every other node directly with zarr.open -- and File is a handle class carrying write methods (addLink, writeRefs, setRefAttr) that a read-only reader has no business holding. Open the root with zarr.open, like every other node in the class, and read the spec location from the root attributes. The ".specloc" key is not a valid MATLAB identifier, so jsondecode renames it; io.internal.zarr3.getSpecLocAttributeName derives that name once and is shared with convertAttributes, which previously computed it separately. The reader now depends only on hdmf.zarr.Reference, hdmf.zarr.Link and hdmf.zarr.isReferenceArray, which ensureAvailable probes accordingly. The test fixture still uses hdmf.zarr.File to write conventions, which is what File is for. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The Zarr3 reader carried a containers.Map from every compound field name to its hdmf-zarr "zarr_dtype" value, but both consumers only ever asked one question of it: is this field tagged "object". Every other entry restates the field's own Zarr v3 sub-dtype and was ignored. Replace the map with the set of field names it was really used for. getCompoundFieldSemantics becomes getObjectReferenceFields and returns a (1,:) string; Zarr3LazyArray's FieldSemantics property becomes ObjectReferenceFields, and the isKey/compare pairs collapse to ismember. The argument now carries real validation where it previously had none, and the property comment shrinks because the name states what the value is. hdmf-zarr writes no "zarr_dtype" marker other than "object", so the map held no second case to serve. Should one appear, a sibling property is a better fit than a map, since a different marker would need a different decode path rather than the same generic lookup. Add unit tests for both internal helpers. They take plain structs, so they need no Zarr store and run where zarr-matlab is unavailable and the fixture-backed Zarr3 tests skip. Fixtures mirror shapes observed from a real TimeSeriesReferenceVectorData column and from zarr.internal.dtype_info rather than assumed ones. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Reading any Zarr v3 TimeSeries loaded its entire data array. For a sparse dataset this failed outright: a 1e6 x 1e6 store asked MATLAB for a 7450.6GB array and hit the array size limit. Two things combined. types.util.checkDtype samples a dataset with load(1) to learn the class of an "any" dtype, and Zarr3LazyArray's tryBuildRegularSelection rejected a lone subscript, whose fallback is a full readAllData. So the one indexing form the type probe uses was the one form the backend did not serve. Handle linear indexing directly: convert the indices with ind2sub and read the elements, mirroring the point read the HDF5 backend performs with H5S_SELECT_ELEMENTS. Duplicates are deduplicated before reading and restored afterwards, so the result matches indexing the loaded array for unordered, duplicated and column selections alike. Compound datasets keep the previous path, since their records are assembled into a table that an element-wise read does not reproduce. Also stop the probe from doing I/O at all. validateAnyType inspects only the class of a value, so a non-compound DataStub is validated from its declared dataType instead of a sample element. This is scoped to that one call site: unwrapValue's other callers consume the value rather than just its class. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A compound dataset with a text field could not be read at all. Reading
hdmf-zarr's HERD.entities failed with
Field 'entities.entity_id' has type mismatch: expected 'char' but
DataStub has 'string'
zarr-matlab represents both Zarr text types, "string" and
"fixed_length_utf32", as a MATLAB string, whereas the HDF5 backend
reports H5T_STRING as char and the generated type classes declare such
fields as 'char'. types.util.checkDtype compares compound descriptor
entries by name, so the two backends disagreeing on the name of the
same logical dtype made an otherwise valid file unreadable.
Report text as char and convert the values to cellstr on read, the
shape io.parseCompound produces for an HDF5 compound's variable-length
string field. A compound dataset now reads back as the same MATLAB
types whichever backend wrote it, and the descriptor comparison stays
strict rather than being relaxed to tolerate the difference.
The fixture gains a compound array with text fields, since the
existing pixel_mask column is numeric and could not cover this.
Note the same divergence remains for non-compound text, where the
value is coerced by correctType and so does not break the read.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Implement Reader.isReferenceDataset for the Zarr v3 backend, so that types.untyped.ExternalLink.deref can decide how to handle a linked dataset without knowing how the backend encodes references. Zarr v3 has no native reference type. hdmf-zarr marks such a dataset with a zarr_dtype of "object", which io.internal.zarr3.buildNodeInfo already surfaces as the node's Datatype, so this reports what readDatasetValue was testing inline; readDatasetValue now calls the method, leaving one place that decides. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The fixture's external link named a store that was never created, so it only ever proved the link record could be read. Reading a link and following one are different things, and only the first was covered. Create a second store for links to point at, holding the two node kinds deref tells apart without any neurodata type having to be generated: a plain dataset, which yields a lazy stub, and an untyped group, which deref rejects by name. The links record an absolute source so that following them does not depend on the process working directory. Both tests fail without the code they cover: removing Zarr3Reader.isReferenceDataset breaks the dataset case, and requiring the HDF5-only 'Datatypes' field again breaks the group case with UnknownNodeType instead of UntypedGroup -- which is what makes the second test evidence that the node was classified as a group at all. The relative-source link is left as it was, since following one is still subject to the working-directory resolution reported in #865. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The Zarr suites fail on R2021a, where zarr-matlab uses a MATLAB class introduced in R2021b. Filter them by release as well as by whether the packages are installed, so an unsupported release reports a skip with a reason rather than a failure. The floor is zarr-matlab's stated requirement, R2022b, rather than R2021b where the suites happen to pass today. Passing on a release the dependency disclaims is not something anyone committed to: a later zarr-matlab could use an R2022b feature in a new code path, well within its own contract, and that would surface here as an unexplained MatNWB regression. If the requirement is more conservative than it needs to be, lowering it in zarr-matlab is the change that should drive this one. The release is checked before the packages so that an old release reports itself as the reason rather than "packages not found". Tests that exercise MatNWB's own zarr3 code without loading zarr-matlab -- the io.internal.zarr3 helpers, the backend factory's error paths -- are deliberately left ungated and keep running on every release. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The Zarr v3 patch left 61 lines uncovered, in three groups: backend validation errors (BackendFactory store detection, ensureAvailable's missing-dependency error, external-reference rejection), lazy-indexing logic no fixture reached (rank >= 3 axis reversal, compound partial reads, reference-field decoding, linear indexing into 1-D and row-vector datasets, ':' selections, scalar-marked datasets), and rare-input branches (checkDtype's unresolvable-declared-type fallbacks, scalar-cell unwrap, non-struct attributes). Extend the shared fixture with the missing shapes -- a rank-3 dataset, a TimeSeriesReferenceVectorData-style compound with an "object"-tagged reference field, a zarr_dtype:"scalar" dataset, a zero-length dataset and a rank-0 variable_length_bytes blob -- and add direct unit tests for each path. The BackendFactory tests use handwritten minimal stores (a folder plus one zarr.json), so they run on every MATLAB release with no zarr-matlab dependency. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Implement Reader.getExternalLinkBase for the Zarr v3 backend. The method was added on main by #871, which resolves a relative external link target against a backend-supplied base instead of the process working directory; io.parseGroup asks the reader for that base while constructing every link, so without an implementation any Zarr store containing links stopped reading at all after the rebase onto main. The base is the store path itself, not the store's parent. hdmf-zarr computes the stored source with os.path.relpath against the store path (see backend.py and the discussion on #865): a store is a directory, so the relpath(target, file) formula HDF5 tooling uses gains one extra "..". An implementation copying HDF5's parent-directory base would resolve every relative Zarr link one directory too high, which is what the new relative-link fixture entry pins down -- its ".." only finds the sibling target store when resolved against the store path, from a working directory that is neither the store nor its parent. With this, processed_data.nwb.zarr from the PyNWB tutorial output -- two stores linked by hdmf-zarr's own relative source -- reads from any working directory. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
ehennestad
force-pushed
the
zarr-support/5-zarr3-reader
branch
from
August 31, 2026 14:12
bd8d2ce to
4258c80
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
Add reader for Zarr v3 files.
Needs: https://github.com/catalystneuro/zarr-matlab
How to test the behavior?
Todo
Checklist
fix #XXwhereXXis the issue number?