fix: resolve chunks=False to chunk size 1 on zero-length axes - #4328
Merged
d-v-b merged 3 commits intoSep 9, 2026
Conversation
`chunks=False` means "one chunk covering the whole array", but on a zero-length axis it built a chunk of size 0. For Zarr format 3 this was rejected by RegularChunkGridMetadata with "integer chunk edge length must be >= 1, got 0"; with shards="auto" it raised ZeroDivisionError on both formats; and for Zarr format 2 it silently wrote `chunks: [0]`, which corrupted reads after the axis was later resized. Route `False` through the `-1` sentinel so the zero-length clamp added in zarr-developers#4307 (max(span, 1), matching _guess_regular_chunks) lives in one place. Both spellings now yield chunk size 1 on empty axes. Rebased over zarr-developers#4218, which rewrote normalize_chunks_nd to build FixedDimension/VaryingDimension grids: the False branch now falls through to the -1 path instead of returning a ChunkGrid directly, and the test table expectations use the new bare-int / tuple form. Assisted-by: ClaudeCode:claude-fable-5-1
Assisted-by: ClaudeCode:claude-fable-5-1
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4328 +/- ##
=======================================
Coverage 94.31% 94.31%
=======================================
Files 92 92
Lines 12920 12920
=======================================
Hits 12185 12185
Misses 735 735
🚀 New features to boost your workflow:
|
d-v-b
marked this pull request as ready for review
September 9, 2026 17:42
Contributor
Author
|
this is a simple bugfix, so i'm self-merging |
7 tasks
d-v-b
added a commit
to d-v-b/zarr-python
that referenced
this pull request
Sep 9, 2026
…, 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
d-v-b
added a commit
to d-v-b/zarr-python
that referenced
this pull request
Sep 9, 2026
…, 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
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
1 participant
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 AI-authored PR ensures that
chunks=False(a pattern I dislike very much, but which we keep around for backwards compatibility), doesn't create 0-length chunks when the array shape is 0.🤖 AI text below 🤖
Summary
chunks=Falsemeans "one chunk covering the whole array", but on a zero-length axis it produced a chunk of size 0. For Zarr format 3 this was rejected byRegularChunkGridMetadata(integer chunk edge length must be >= 1, got 0); withshards="auto"it raisedZeroDivisionError; and for Zarr format 2 it silently wrotechunks: [0], which corrupted reads once the axis was appended to and resized — the degenerate grid reports zero chunks, so reads return uninitialized memory or zeros.#4307 already fixed the equivalent
chunks=-1sentinel by clamping tomax(span, 1), matchingchunks="auto". This PR routes theFalsebranch ofnormalize_chunks_ndthrough that same-1path so there is a single clamp; both spellings now yield chunk size 1 on an empty axis.The #4307 regression test is extended to cover
Falsealongside-1for(0,),(0, 4)and(4, 0)on both formats, with and withoutshards="auto"(and with a shard-size budget), asserting the in-memory chunks/shards, the stored chunk grid, and thatappend/resizealong the empty axis round-trip data. Without the source change exactly the 14Falseparametrizations fail and every-1case passes.Found during the pre-release review for #4256.
For reviewers
The source change is a two-line fallthrough (
chunks = -1) replacing the directFixedDimensionconstruction in theFalsebranch, on top of #4218's chunk-grid rewrite. The v2 behavior change is intentional: the previouschunks: [0]metadata was invalid and returned wrong data after a resize, and no existing test depended on it.Author attestation
TODO
docs/user-guide/*.md(n/a)changes/