fix: resolve chunks=-1 to chunk size 1 on zero-length axes - #4307
Merged
d-v-b merged 2 commits intoSep 2, 2026
Conversation
The -1 chunk sentinel resolved to chunk size 0 on zero-length axes, which broke every downstream sharding path differently: a ValueError with shards=None, a ZeroDivisionError with shards="auto", and an infinite loop with shards="auto" plus array.target_shard_size_bytes. Clamp it to 1, matching the auto-chunking clamp in _guess_regular_chunks. Also guard _guess_num_chunks_per_axis_shard against zero-size chunk shapes directly (same non-terminating-loop cause as the 0-d case fixed in zarr-developers#4305), and fix the arithmetic in its docstring example. Follow-up to zarr-developers#4305 / issue zarr-developers#4304. Assisted-by: ClaudeCode:claude-fable-5-1
Assisted-by: ClaudeCode:claude-fable-5-1
d-v-b
force-pushed
the
fix/shard-guess-zero-size-chunks
branch
from
September 2, 2026 12:01
873e1df to
f850df5
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4307 +/- ##
=======================================
Coverage 94.21% 94.21%
=======================================
Files 92 92
Lines 12863 12863
=======================================
Hits 12119 12119
Misses 744 744
🚀 New features to boost your workflow:
|
d-v-b
marked this pull request as ready for review
September 2, 2026 13:25
Contributor
Author
|
I'm self-merging, because this is a bugfix that doesn't affect public API. |
7 tasks
d-v-b
added a commit
that referenced
this pull request
Sep 9, 2026
* fix: resolve chunks=False to chunk size 1 on zero-length axes `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 #4307 (max(span, 1), matching _guess_regular_chunks) lives in one place. Both spellings now yield chunk size 1 on empty axes. Rebased over #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 * chore: rename changelog fragment to the PR number Assisted-by: ClaudeCode:claude-fable-5-1
7 tasks
d-v-b
added a commit
to d-v-b/zarr-python
that referenced
this pull request
Sep 9, 2026
`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
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
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.
Summary
follow up for #4304, ensures that we handle 0-length axes correctly in auto chunk size logic. written by claude, claude's PR is here
Author attestation
TODO
docs/user-guide/*.mdchanges/