Conversation
d-v-b
force-pushed
the
fix/zero-length-single-invariant
branch
from
September 9, 2026 18:28
7c5688b to
302b638
Compare
…, clamps and metadata Invariant: a chunk edge length is always >= 1; a dimension's extent may be 0, in which case the dimension has zero chunks (ceildiv(0, size) == 0). Zero-length-axis bugs have recurred since 2017 (#150, #241, #303, zarr-developers#972, zarr-developers#1977, zarr-developers#2434, zarr-developers#3711, zarr-developers#4305, zarr-developers#4307, zarr-developers#4328) because the layers disagreed on this invariant and every span-derived chunk spelling clamped on its own: - The metadata layer (common.py, metadata/v3.py) required chunk edges >= 1, but the in-memory FixedDimension allowed size == 0 with four special-case branches left over from zarr-developers#2434, so normalization could build a grid the metadata constructor then rejected. FixedDimension now rejects size < 1 and the four `if self.size == 0` branches are gone. VaryingDimension already required edges > 0 and is unchanged. - `chunks=-1`, `chunks=False`, `chunks="auto"` (_guess_regular_chunks, both the typesize == 0 early return and the np.maximum line) and `shards="auto"` each derived "one chunk covering the axis" independently. They now all go through one helper, `_full_span_chunk_size(span) = max(span, 1)`, which is the single definition of that phrase for a possibly zero-length axis. - Zarr format 2 metadata had no chunk >= 1 check, so a legacy `chunks: [0]` document opened fine and read uninitialised memory after a resize. It now raises a clear ValueError at parse time, matching the format 3 grid. - Rectilinear grids had no creation-time spelling for a zero-length axis: normalize_chunks_1d required sum(edges) == span, which no list of positive edges can satisfy for span 0, even though the same state is reachable via resize((0,)) and round-trips through reopen. For span == 0 any non-empty list of positive edges is now accepted verbatim, producing the same VaryingDimension(edges, extent=0) that resize produces; the strict sum check is kept for span > 0. Tests: the per-spelling regression test from zarr-developers#4328 is replaced by one matrix over {-1, False, "auto", 1, (1,...), [[2, 2]]} x {(0,), (0, 4), (4, 0), (0, 0), ()} x {v2, v3} x {no shards, shards="auto" with and without a byte budget, explicit shards}, with separate small tests for each error case. Tests that constructed FixedDimension(size=0) now assert it raises, and a zero-extent test covers the behaviour the old special cases were guarding. Assisted-by: ClaudeCode:claude-fable-5-1
zarr-python 2.18.7 writes `chunks: [0]` for `zarr.zeros((0,), chunks=False)` and for `chunks=(0,)`, so stores with that document exist. Rejecting them at open would turn a previously-readable array into an error; leaving the 0 in place read uninitialised memory after a resize. Normalize the edge to 1 with a ZarrUserWarning instead — the same grid every other "one chunk spans the axis" spelling produces — and keep rejecting a zero edge on an axis that has data. Assisted-by: ClaudeCode:claude-fable-5-1
Assisted-by: ClaudeCode:claude-fable-5-1
Measured against zarr 2.18.7: `zeros((0,), chunks=False)`, `chunks=-1` and `chunks=(0,)` all write `chunks: [0]`, after which nchunks, read, write, append, resize and reopen-then-read every raise ZeroDivisionError. There was never a working behaviour to preserve; normalizing the edge to 1 makes such arrays usable for the first time. Say so in the comment and fragment instead of claiming the stores were previously readable. Assisted-by: ClaudeCode:claude-fable-5-1
d-v-b
force-pushed
the
fix/zero-length-single-invariant
branch
from
September 9, 2026 18:28
302b638 to
047a92e
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4334 +/- ##
=======================================
Coverage 94.22% 94.23%
=======================================
Files 92 92
Lines 12942 12962 +20
=======================================
+ Hits 12195 12215 +20
Misses 747 747
🚀 New features to boost your workflow:
|
Member
|
I was slightly ahead of you here - I think as long as I release my VZ PR before you release this then VZ at least will not break as a result of this change in Zarr-python. |
… format 3
The compatibility policy for a stored chunk size of 0 on a zero-length
axis covered Zarr format 2 only, so arrays written by zarr-python 3.0 and
3.1 with `chunk_shape: [0]` — or `[false]`, which 3.0 wrote for
`chunks=False` — still could not be opened at all.
`ArrayV3Metadata` now applies the same policy to a regular chunk grid: a
stored chunk size of 0 on a zero-length axis is read as 1 with a
`ZarrUserWarning`, and a zero chunk size on a positive-length axis is
left for the chunk grid parser to reject. It runs in `__init__` rather
than in the grid parser because the policy needs the array shape, which
chunk grid metadata does not carry.
Both warnings now say how to store a corrected chunk size — open the
array writable and call `array.update_attributes({})`, which rewrites the
whole document from the parsed metadata — from one shared constant. The
Zarr format 2 warning also named only zarr-python 2.x; measured against
real installs, every 3.x release before 3.4 wrote a zero chunk size for
an empty array too (3.3.0 for `chunks=-1` and `chunks=False`).
Tested against stores written by zarr 3.0.10, 3.1.6 and 3.3.0: they open,
append without losing data, and re-save to a chunk size that reopens
without a warning.
Assisted-by: ClaudeCode:claude-opus-5
… in one routine The legacy zero-chunk policy was written out twice: inline in `ArrayV2Metadata.__init__`, and again in a Zarr format 3 helper. The V2 copy also zipped with `strict=False` and re-appended any trailing chunk entries only so that a separate length check, `parse_metadata`, could report a dimensionality mismatch after construction. `parse_stored_chunk_shape` in `zarr.core.metadata.common` is now the one place a stored chunk shape is checked against its array's shape, for both formats: one entry per axis, every integer chunk size at least 1, and a size of 0 (or JSON `false`) on a zero-length axis read as 1 with a warning that names the writer and how to re-save. Non-integer entries, such as edge lists, pass through for the caller's own parser. `ArrayV2Metadata.__init__` calls it directly and `parse_metadata` is gone. The Zarr format 3 adapter only locates a regular grid's `chunk_shape` in the stored document and hands it over; it still runs in `ArrayV3Metadata.__init__` because chunk grid metadata has no array shape. The Zarr format 3 import changes that existed only for the old helper are reverted. Tests for the policy now target the routine: one table of valid and legacy inputs, and one test per rejection (dimension mismatch, zero on a non-empty axis, negative). They replace metadata-level tests in test_v2.py and test_v3.py that only re-tested the same rules; the end-to-end tests still cover both formats' wiring against stored arrays. Assisted-by: ClaudeCode:claude-opus-5
…nk grids `parse_stored_chunk_shape` passed non-integer entries through "for the caller's own parser", which made a regular-grid policy look like a general chunk shape routine and let it decide what a 0-length chunk means for grids it does not own. A rectilinear grid, or any other grid, is free to define its own semantics for 0-length chunks. It is now `parse_stored_regular_chunk_shape`, typed `Sequence[int]`, with no pass-through, and its docstring says it applies to Zarr format 2 `chunks` and Zarr format 3 `regular` grids only. The Zarr format 3 caller hands it a chunk shape only when the grid is named `regular` and every entry is an integer (`_is_regular_chunk_shape`); anything else is not a regular chunk shape and goes to the chunk grid parser untouched. Assisted-by: ClaudeCode:claude-opus-5
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.
This PR tightens our representation of chunk grids to disallow the representation of 0-length chunks. This has compatibility implications: previous versions of
zarrcould access arrays with 0-length chunks and exercise a subset of the Zarr API against them (reading / writing attributes), but not write chunks. I'm still thinking about the right compatibility shim for this, hence the draft status,Previous PRs that removed 0-length chunks caused issues downstream so I'd like some eyes on this change (cc @TomNicholas)
🤖 AI text below 🤖
Makes chunk sizes positive while allowing zero-length array axes.
FixedDimensionnow rejectssize=0, and full-span chunk inference uses a shared helper returningmax(span, 1). Rectilinear creation accepts a non-empty list of positive chunk sizes on an empty axis and retains those sizes for later growth.Compatibility policy
A stored chunk size of 0 on a zero-length axis is read as 1 with a
ZarrUserWarning. A zero chunk size on a positive-length axis is rejected. This is a compatibility policy for the current grid model; the metadata does not prove whether chunk payloads exist in the store.This now covers both formats. Zarr format 3 was added after measuring what older releases actually wrote — before, arrays written by 3.0 and 3.1 could not be opened at all.
Both formats get the policy from one routine,
parse_stored_regular_chunk_shapeinzarr.core.metadata.common, which checks a stored regular chunk shape against its array's shape: one entry per axis, every chunk size at least 1, and the empty-axis exception above. It is regular-grid-specific by design: another chunk grid, such as the rectilinear grid, is free to define its own semantics for 0-length chunks, so its chunk sizes are never passed to it.ArrayV2Metadata.__init__calls it directly, which also replaces V2's separate after-the-fact length check (parse_metadata). For Zarr format 3 it is called fromArrayV3Metadata.__init__rather than from the chunk grid parser, because chunk grid metadata does not carry the array shape, and only for a grid namedregularwhosechunk_shapeis all integers; anything else goes to the chunk grid parser untouched.What older releases wrote
Measured with real installs, creating an array with a zero-length axis:
chunks[0]chunks=-1andchunks=False, 3.0–3.2 forchunks=(0,)/Falsechunks[false]chunks=Falsechunk_shape[0]chunk_shape[false]chunks=FalseSo this is not only about zarr-python 2.x: 3.3.0 wrote a zero chunk size for an empty Zarr format 2 array a few weeks ago, and 3.4.0 still reads those stores.
Why this matters beyond validation hygiene
Appending to such a Zarr format 2 array loses data silently in 3.4.0. Taking an empty array created by 3.3.0 with
chunks=-1and appending[7, 8, 9]with 3.4.0: the array reportsshape=(3,), no chunk file is written, and it reads back[0, 0, 0]with no error. With this PR the same store opens with a warning, appends correctly, and re-saves to valid metadata.Zarr format 3 stores with a zero chunk size fail to open in 3.4.0 rather than losing data, so that half is a loud failure being fixed, not a silent one.
Re-saving corrected metadata
Both warnings say how to persist the corrected chunk size: open the array writable and call
array.update_attributes({}), which rewrites the whole metadata document from the parsed metadata. Verified for both formats against real legacy stores: the document is rewritten with chunk size 1 and reopening is warning-free.Tests
Tests cover six chunk spellings over empty/scalar shapes and applicable format/sharding combinations; separate cases check invalid chunk edges and positive-extent rejection. The policy itself is tested on
parse_stored_regular_chunk_shape: one table of valid and legacy inputs (multi-axis shapes, JSONfalse, numpy integers) and one test per rejection (dimension mismatch, zero on a non-empty axis, negative). Wiring is tested end to end: a round-trip test over both formats and both stored spellings (0and JSONfalse) that opens a legacy store, appends, checks the data survives, re-saves withupdate_attributes({})and asserts a clean reopen, plus a rejection test for a stored zero chunk size on a positive-length axis in both formats. The Zarr format 3 cases were confirmed to fail without the shim. Full suite: 8904 passed.Merge and release notes
main. Merges cleanly with fix: make the chunk normalizer the one judge of a chunk specification #4376 and test: sample the full rectilinear chunk grid declaration space #4377; with fix: read mixed regular/rectilinear chunk grids written by zarr 3.2.x #4375 there is a one-line import conflict inmetadata/v3.py(adjacent import lines; keep both). All four together, with that resolved: 8953 tests pass, and a real zarr 3.2.1 mixed-grid store (Mixed regular/rectilinear chunk grids written by 3.2.x are unreadable by 3.3+ #4374) still reaches fix: read mixed regular/rectilinear chunk grids written by zarr 3.2.x #4375's reader because it is not a regular chunk shape.🤖 Generated with Claude Code