[Zarr-Support] Add Zarr v2 reader support - #821
Draft
ehennestad wants to merge 10 commits into
Draft
Conversation
Contributor
|
Just FYI, there are also ongoing PR's in hdmf-zarr to adopt Zarr V3 https://github.com/hdmf-dev/hdmf-zarr/pulls |
Contributor
|
Sorry, I only meant to add a comment and not close the PR, my mouse was a few pixels too far too the left |
Remove debugging scaffolding that wrapped io.createParsedType and reader.readDatasetValue in try/catch blocks which suppressed errors (returning [] or missing) and only warned. This affected every backend, not just Zarr, and silently weakened read validation: nwbRead no longer failed on malformed files. Restoring error propagation fixes tests.unit.nwbReadTest/readFileWithInvalidTimeSeriesTimestampsShape. The warn-on-read-failure behavior is being designed in a separate PR and is preserved on the debug/read-error-warning branch. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Adds a pynwb+hdmf-zarr Python script that writes a minimal NWB Zarr v2 store exercising all reader paths (schema version, embedded specs, soft link, object-reference attribute, object-reference dataset, scalar string, 2-D float32 array). createZarrTestFile.m invokes it via pyrunfile to produce the fixture on demand in a temporary folder. Adds three test utilities: - getZarrWrapperPath: resolves ZARR_WRAPPER_PATH env var or the default external_packages location used by setup.m - isPythonModuleAvailable: probes a Python module via importlib without importing it, returning false on any error - assumeZarrSupport: combines the three prerequisite checks (wrapper path, tensorstore, hdmf_zarr) and skips the calling test if any is absent Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replaces hardcoded developer paths with a TestClassSetup that generates the Zarr fixture on-the-fly via createZarrTestFile and applies PathFixture and TemporaryFolderFixture. Tests skip gracefully via assumeZarrSupport when the wrapper or Python packages are absent. Assertion values are derived from the constants in generateZarrTestFile.py (IDENTIFIER, NUM_ELECTRODES, NUM_SAMPLES) so the test and fixture stay in sync without magic literals. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
setup.m clones the MathWorks Zarr wrapper (ehennestad fork, pinned to a55a3f2a) into external_packages/ and installs tensorstore into MATLAB's configured Python interpreter. CI changes (run_tests.yml): - Checks out the Zarr wrapper at the pinned SHA and exports ZARR_WRAPPER_PATH so MATLAB tests can locate it without a path search. - Installs tensorstore and hdmf-zarr on Python 3.10+ rows via a best-effort pip step; tests skip gracefully if unavailable. .gitignore: excludes external_packages/MATLAB-support-for-Zarr-files so the user-installed wrapper is never accidentally committed. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…sets Fixture additions: - SPIKE_TIMES, NUM_SPIKE_TIMES, ELECTRODE_LOCATION constants - channel_id integer column on the electrode table (1-D int64 dataset) - Explicit SPIKE_TIMES split across units (fixture stays deterministic) New Zarr2ReaderTest cases: - read1dDatasetReturnsDataStub: verifies scalar dims for /units/spike_times - readStringArrayDatasetContainsExpectedValues: verifies electrode location strings load correctly regardless of whether the reader returns a DataStub or a direct cell/string array New Zarr2LazyArrayTest cases: - integer1dDatasetHasCorrectMatlabType: verifies int64 dtype mapping through getMatlabDataType for the channel_id electrode column - loadWithInfCountReadsToEnd: exercises the Inf-count branch in readPartialData which computes count from dims/start/stride rather than taking it literally - loadMatStyleIrregularSelectionFallsBackToFullRead: non-uniform step sizes disable the partial-read path; result must match full-read-then-index Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Updates the pinned MathWorks Zarr wrapper SHA from a55a3f2a to 66373602 (phase7-compound-dataset-support). That commit adds compound dataset read support to the wrapper's zarrread/ZarrDatatype — required by the getMatlabDataType compound path in matnwb. The v2 numeric read path is unchanged (v3 additions are guarded by zarr_format checks). Fixture: adds a PlaneSegmentation with NUM_ROIS=3, NUM_PIXELS_PER_ROI=4 producing a 12-record pixel_mask compound dataset (x uint32, y uint32, weight float32) at /processing/ophys/PlaneSegmentation/pixel_mask. New Zarr2ReaderTest case readCompoundDatasetReturnsTable verifies that readDatasetValue returns a DataStub, that load() returns 12 records, and that the result (table or struct array) exposes x, y, and weight fields. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Removed the channel_id electrode column — pynwb's auto-generated id column (int64) is a cleaner source for the integer dtype test and requires no extra fixture machinery. Updated Zarr2LazyArrayTest to point at electrodes/id and assert 'int64'. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
ehennestad
force-pushed
the
zarr-support/4-zarr2-reader
branch
from
July 4, 2026 09:07
68a9742 to
34c5881
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
2 participants
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
This PR adds a Zarr v2 reader implementation for
nwbRead.Depends on: https://github.com/ehennestad/MATLAB-support-for-Zarr-files/tree/phase7-compound-dataset-support
How to test the behavior?
Checklist
fix #XXwhereXXis the issue number?🤖 Generated with Codex