Support zarr 3 alongside zarr 2 for non-ragged zspy data - #539
Draft
CSSFrancis wants to merge 2 commits into
Draft
CSSFrancis wants to merge 2 commits into
CSSFrancis wants to merge 2 commits into
Conversation
zspy is currently pinned to zarr<3 because several of the APIs it uses are v2-only. Port them, keeping v2 working, and widen the pin to zarr>=2,<4. What had to change: - Store selection: NestedDirectoryStore is v2-only, so use LocalStore under v3. zarr 3's ZipStore defaults to read-only, so it needs its mode up front rather than only via open_group. - Store detection: zarr 3 stores derive from zarr.abc.store.Store rather than MutableMapping, so "a store was passed instead of a path" has to test for both. - The close-file check referenced zarr.ZipStore/DBMStore/LMDBStore as bare top-level names. Those don't exist under v3 -- DBMStore and LMDBStore were dropped rather than renamed -- and it threw on every call regardless of store type. Build the tuple from what's available. - Group.require_dataset became require_array, without `exact` and taking a list of zarr's own codecs. Add a _require_dataset hook on the writer base class so all of this stays inside the zspy backend and the h5py path is untouched, and translate classic numcodecs compressors rather than making callers pass a different type per zarr version. Note the translation targets zarr.codecs.numcodecs, since numcodecs.zarr3 is now deprecated. - The retry loop in overwrite_dataset relied on `del group[key]` raising KeyError to escape. zarr 3 deletes a missing key silently, so a TypeError from the request itself looped forever -- rsciio hung rather than reporting the error. Retry once, then propagate. Ragged arrays are not ported: they rely on zarr 2 object codecs (VLenArray, MsgPack) with no v3 equivalent. Writing one under zarr 3 now raises an explanatory NotImplementedError, and reading a file whose ragged arrays were written under zarr 2 raises an explanatory ValueError instead of zarr's raw "No Zarr data type found" error. Add a -zarr3 CI leg, since zarr 2 and 3 can't coexist in one interpreter.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #539 +/- ##
==========================================
- Coverage 88.78% 88.75% -0.04%
==========================================
Files 113 113
Lines 13113 13148 +35
Branches 2421 2433 +12
==========================================
+ Hits 11643 11669 +26
- Misses 922 927 +5
- Partials 548 552 +4 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
zarr 3 defaults to writing zarr *format* 3 metadata (zarr.json). A user who upgraded zarr and saved a .zspy would produce a file that anyone still on zarr 2 cannot open at all -- it fails with an unrelated-looking "nothing found at path ''" -- which is exactly the ecosystem split the original zarr<3 pin was there to avoid. zarr 3 reads format 2 without trouble, so pinning the written format to 2 keeps files interchangeable in both directions regardless of which zarr the writer happens to have. Verified round-trip: a file written under zarr 3.2.1 now reads correctly under zarr 2.18.7. Writing format 2 also means the codec pipeline takes classic numcodecs codecs directly, so the compressor no longer needs translating to zarr 3 codec wrappers -- which drops the dependency on numcodecs.zarr3, now deprecated in favour of zarr.codecs.numcodecs.
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.
Towards #352. Draft: the ragged-array half is deliberately not included
(see below), and I'd like agreement on that split before going further.
Description of the change
zspyis pinned tozarr<3(#351) because several of the APIs it usesare v2-only. This ports them while keeping zarr 2 working, and widens the
pin to
zarr>=2,<4.Verified by installing this branch against zarr 3.2.1 — plain
round-trip,
compressor=None,store_type="zip"and lazy loading allpass, and the whole
test_zspy.pysuite is green under both zarr 2 (24passed) and zarr 3 (17 passed, 7 skipped as v2-only).
What had to change
NestedDirectoryStoreis v2-only →LocalStoreunder v3. zarr 3's
ZipStoredefaults to read-only, so it needs itsmode up front rather than only via
open_group.zarr.abc.store.Store,not
MutableMapping, soisinstance(filename, MutableMapping)nolonger recognises "a store was passed instead of a path".
zarr.ZipStore/zarr.DBMStore/zarr.LMDBStoreas bare top-level names. Under v3 those don't exist —DBMStore/LMDBStorewere dropped, not renamed — so it raisedAttributeErroron every call regardless of store type. Now builtfrom whatever the installed zarr provides.
Group.require_dataset→require_array, which has noexactandtakes a list of zarr's own codecs. Added a
_require_datasethook onthe writer base class so all the version branching stays inside the
zspy backend and the h5py path is untouched. Classic
numcodecscompressors are translated rather than making callers pass a different
type depending on their zarr version, so the documented
compressor=parameter keeps working.
overwrite_datasetrelied on
del group[key]raisingKeyErrorto terminate. zarr 3deletes a missing key silently, so a
TypeErrorcoming from therequest itself (e.g. the v2-only compressor) looped forever — rsciio
hung instead of reporting the error. It now retries once and
propagates. This one is worth a look even independently of zarr 3.
Note on the plan's recommendation: it suggests
numcodecs.zarr3forthe compressor wrappers. That module is now deprecated ("Import Blosc via
zarr.codecs.numcodecs.Bloscinstead. This requires Zarr Python >=3.1.3"), so this targets
zarr.codecs.numcodecswith a fallback.What is not included: ragged arrays
Ragged arrays (variable-length markers, diffraction vectors) rely on zarr
2's object codecs —
VLenArray,MsgPack— which have no v3 equivalent.The known workaround is packing each cell to bytes ourselves and storing
via
VariableLengthBytes, but that type carries anUnstableSpecificationWarning: its on-disk identifier is flagged upstreamas likely to change. Committing to it is a real format-stability decision
and I don't think it should ride along in this PR.
So under zarr 3 both directions now fail with an explanation instead of an
obscure error:
NotImplementedErrornaming the cause andsuggesting
zarr<3or.hspyValueErrorexplaining that zarr 3 cannot read them, instead of zarr'sraw
No Zarr data type found that matches {'name': '|O', 'object_codec_id': 'vlen-array'}The second is worth emphasising: it is zarr's own v2-compat metadata
resolution, so no write-side fix makes existing marker/vector
.zspyfiles readable under zarr 3. That is the main argument for widening the
pin rather than jumping to a hard
>=3floor — zarr 2 has to stay aworking fallback.
Worth noting #536 (CSR encoding for vector-shaped ragged arrays)
composes with this: because it stores vectors as plain concrete-dtype
arrays, such data needs none of the object-codec machinery and already
round-trips under zarr 3. If both land, the remaining gap narrows to
genuinely heterogeneous ragged data.
Known limitation, needs a hyperspy-side fix
Passing a zarr store object (rather than a path) to
save()/load()does not work under zarr 3: hyperspy detects that case with
isinstance(filename, MutableMapping)(hyperspy/io.py:898), so a zarr 3store is treated as a path and raises
TypeErrorbefore rsciio isreached. Path-based use is unaffected. The corresponding test is skipped
under zarr 3 with a comment.
Progress of the PR
upcoming_changesfolder-zarr3CI leg, since zarr 2 and 3 can't coexist in one interpreter)