[h-mayorquin]

tensorstore rejects the string arrays that every NWB file has, and in #1918 I asked whether zarr-python could do the validation instead. This PR validates Zarr v3 arrays with zarr-python, drops tensorstore, and moves zarr to a zarr extra so only validating Zarr assets needs pip install "dandi[zarr]". I also open Zarr v2 stores without consolidated metadata as zarr-python 3 can't parse some arrays that hdmf-zarr wrote with zarr-python 2, and raised the nwbinspector floor to 0.7.2, the first release that does not require hdmf-zarr. This should close #1918.

I made zarr an extra because of the concern @yarikoptic raised in the DANDI meeting on September 28 about forcing upgrades on users who don't need the new zarr. With the extra, pip install dandi does not install zarr at all, and validating a Zarr asset without it gives an error that says to install dandi[zarr]. My preference is still a regular dependency with a floor of 3. zarr 3 runs on Python 3.11 and zarr-python 2 has not had a release since April 2025. So the extra mostly serves environments that pin an unmaintained version, and in exchange everyone who uploads Zarr has one more install step and we have one more code path to maintain. But I am fine with the extra as a first step if that is easier to agree on.

Finally, this needs an upgrade of nwbinspector to 0.7.2, the first release that does not require hdmf-zarr, and I am not sure if you want to handle that separately.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.92%. Comparing base (152e941) to head (2f9e729).

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1971      +/-   ##
==========================================
+ Coverage   90.83%   90.92%   +0.09%     
==========================================
  Files          92       92              
  Lines       14190    14220      +30     
==========================================
+ Hits        12889    12930      +41     
+ Misses       1301     1290      -11     

Flag	Coverage Δ
unittests	90.92% <100.00%> (+0.09%)	⬆️

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.

[h-mayorquin]

Actually, I want to open a new PR for the inspector only but...

[yarikoptic]

tensorstore rejects the string arrays that every NWB file has,

is that something to bring up with tensorstore? if that is permitted by Zarr v3 but they reject, is that just an omission or conscious decision to validate additional restrictions/additions beyond it?

[h-mayorquin]

tensorstore rejects the string arrays that every NWB file has,

I think it is an omission. tensorstore does not support variable-length data types at all, and there is an issue about it open since 2023:
google/tensorstore#103

That issue is about Zarr v2 but the limitation is the same, and one of the maintainers said there that supporting variable-length data types "will be quite involved". In Zarr v3, string is an extension data type outside the core spec:
https://github.com/zarr-developers/zarr-extensions/tree/main/data-types/string

So I don't think there is anything new to bring up with them.

For the inspector, I opened #1972 as a draft because of the limit above.

[yarikoptic]

Actually, I want to open a new PR for the inspector only but...

didn't know about such feature! boosted to 5 and added you to "exclusion list" . FTR: URL https://github.com/dandi/dandi-cli/settings/interaction_limits

[yarikoptic]

Would we be ok validating zarr v2 as well?

[h-mayorquin]

Yes, zarr-python 3 reads Zarr v2 stores. I checked it with an NWB file written in Zarr v2 by hdmf-zarr, and the existing Zarr v2 tests run with zarr-python 3 in this PR. That is what the use_consolidated=False change below is for.

[yarikoptic]

"not sure" on reasoning here as we are changing behavior for which we did not have an issue reported.

[h-mayorquin]

I don't understand the quotes in "not sure", did I write a comment there?

This restores what dandi did with zarr-python 2, where zarr.open never reads .zmetadata:
https://github.com/zarr-developers/zarr-python/blob/3ad97b9c783fecef6e9528c41e44aad417ca2447/zarr/convenience.py#L44-L139

zarr-python 3 reads it by default and can't parse some of the metadata that hdmf-zarr wrote with zarr-python 2. This is something that broke once Kabi's #1653 got merged, we just do not have a test for that and people do not upload too many NWB datasets I guess. I added a test here that fails on the master branch. I am happy to open an issue but the fix is basically here.

It fits here as this PR makes zarr-python 3 required. On master it would require something like this, as use_consolidated does not exist in zarr-python 2:

kwargs = {"use_consolidated": False} if Version(zarr.__version__).major >= 3 else {}
data = zarr.open(str(self.filepath), mode="r", **kwargs)

Then we would need to remove it again when we drop zarr-python 2 in this PR.

[yarikoptic]

"not sure" yet as could we potentially "do both" and just issue a warning if tensorstore pukes for that unsupported reason? I worry that we might be relaxing validation beyond the underlying issue.

[h-mayorquin]

could we potentially "do both" and just issue a warning if tensorstore pukes for that unsupported reason?

Dropping tensorstore was already the plan. The tensorstore functions added in #1614 say:

This function will no longer be needed once the upgrade to zarr-python 3.x is done and should be removed.

dandi-cli/dandi/files/zarr.py

Lines 158 to 159 in 152e941

	This function will no longer be needed once the upgrade to zarr-python 3.x is
	done and should be removed.

I am following that plan and I agree with it for the following reasons.

tensorstore's check is about what tensorstore can compute with and not about what is valid Zarr. The zarr3 driver matches the data type against its own numeric types and anything else throws an error:
https://github.com/google/tensorstore/blob/16f02f78b82762f0f738dbd0d903f745fc3f594a/tensorstore/driver/zarr3/dtype.cc#L48-L110

string is a registered Zarr v3 extension and every NWB file stores its text with it, so right now dandi rejects every NWB file written as Zarr v3.

Doing both has its own costs:

• Two libraries and two code paths for the same check.
• tensorstore stays a dependency. It is a compiled package of 15 to 22 MB and its current Linux wheels need glibc 2.27 or newer.
• The string arrays still need zarr-python to be checked. With only a warning they are not checked at all.

But maybe you have a specific check in mind that tensorstore catches and we need? I put an agent to look over the source code of tensorstore. It is stricter when parsing metadata (for example unknown keys inside a codec or blosc settings out of range) but all the cases it found need hand-edited metadata. On the IBL file from #1918 the only arrays tensorstore rejected were the 113 string arrays which are indeed valid.

Now that I think about it, doing both does not block us, so I am happy to go with it if you want to cover "unknown unknowns". I think keeping more dependencies goes against your own idea of supporting more systems that moved zarr to an extra in the first place (more dependencies is less or equal environments supported).

[yarikoptic]

Thanks for the detail! I just wish there was a proper Zarr and OME-Zarr in particular validation! ATM our validation is ridiculously trivial unfortunately, so by at least testing against two and only ignoring known "non problems" is somewhat of a "non-relaxing" the original amount of validation. May be check out (address?)

• Validate (.ome).zarr against ngff schema using ome-zarr-models-py #1409

while on this trail? ;-) with that, likely we can indeed just drop that .

[h-mayorquin]

Yes! This is what I was thinking about while working on this. First I found pydantic-zarr but it is limited on versions: it requires Python 3.12 and pydantic 2.12 (ome-zarr-models for #1409 also requires Python 3.12, and pydantic < 2.13). But then I found zarr-metadata, the package pydantic-zarr builds on:
https://github.com/zarr-developers/zarr-python/tree/main/packages/zarr-metadata

• It is developed in the zarr-python repository and released on its own as "Spec-defined metadata types, models, and validators for Zarr v2 and v3".
• It needs Python 3.11 and has only two small dependencies, annotated-types and typing-extensions.
• It checks each zarr.json against the spec and the registered extensions and reports every problem with its location, like codecs.1.configuration.level: expected an integer in [0, 9], got 20.
• Together with opening the arrays with zarr-python, it catches the broken metadata that tensorstore caught except tensorstore's own limits and conventions (for example more than 32 dimensions or its dimension_units attribute) and two odd codec combinations.
• The NWB file written by hdmf-zarr passes, string arrays included.
• It is young though, 0.6.0 came out on October 26.

I can give it a try if you think that's useful.