fix: honor rectilinear chunk-grid requests with uniform edges - #4290
Open
CAOShurong wants to merge 1 commit into
Open
fix: honor rectilinear chunk-grid requests with uniform edges#4290CAOShurong wants to merge 1 commit into
CAOShurong wants to merge 1 commit into
Conversation
…evelopers#4272) A nested-sequence chunks spec is an explicit rectilinear request, but since zarr-developers#3899 the stored grid kind was chosen from the edge *values* (is_regular_nd) rather than the form of the request. A spec like [[10, 10, 4]] -- uniform plus a short trailing chunk -- was therefore silently collapsed to a regular grid. The two grids behave identically at creation time but diverge under resize: a regular grid extends the uniform pattern while a rectilinear grid appends an edge, so an append-only workload gets a different (chunk-rewriting) layout from the one it asked for, with no warning and nothing in the metadata recording the substitution. create_chunk_grid_metadata gains a keyword-only requested_rectilinear flag; both v3 creation call sites derive it from the raw user input: a nested-sequence chunks (or shards) spec requests rectilinear, flat and "auto" specs keep inferring from the edge values. This restores 3.2.x semantics (resolve_chunks dispatched on input syntax). Fixes zarr-developers#4272
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4290 +/- ##
==========================================
+ Coverage 94.12% 94.18% +0.06%
==========================================
Files 92 92
Lines 12831 12835 +4
==========================================
+ Hits 12077 12089 +12
+ Misses 754 746 -8
🚀 New features to boost your workflow:
|
7 tasks
d-v-b
added a commit
to d-v-b/zarr-python
that referenced
this pull request
Aug 26, 2026
Drop the regular-grid collapse for explicit list input in normalize_chunks_1d: a per-chunk size list now always produces VaryingDimension, even when the sizes are uniform or uniform plus a short tail. Scalar specs (ints, numpy integers, and the -1 sentinel) still produce FixedDimension, which is the path that makes array creation O(1) in per-dimension chunk count; explicit lists are already O(n) in the input, so nothing is lost. The grid kind now follows the input syntax, matching 3.2.x behavior: previously an explicitly rectilinear spec whose edges happened to look regular was silently stored as RegularChunkGridMetadata, which changes resize semantics — a regular grid grows by extending the uniform pattern while a rectilinear grid appends an edge chunk, breaking append-oriented layouts like (168,) * 13 + (24,). This is resolution option 1 from zarr-developersgh-4272. Uniform dimensions of mixed scalar/list specs still serialize as the spec's bare-int step-size shorthand; explicit lists serialize as edge lists. The touched-chunk-keys assertions in the resize regression test follow the approach from zarr-developersgh-4290, which fixes the same issue on main via a requested_rectilinear flag. Fixes zarr-developers#4272 Co-authored-by: Shurong Cao <CAOShurong@users.noreply.github.com> Assisted-by: ClaudeCode:claude-fable-5
Contributor
|
hi @CAOShurong I think this PR is superseded by #4218, let me know what you think? |
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.
Zarr version
3.3.1.dev34+gd44f9f92 (current
mainat the time of this PR)Numcodecs version
0.16.5
Python Version
3.12.14 (local verification environment)
Operating System
Windows 11 x64
Installation
Editable install from a local clone of
main(d44f9f9), venv via uv.Description
Fixes #4272. A nested-sequence
chunksspec is an explicit rectilinear request, but since #3899 the stored grid kind was chosen from the edge values (is_regular_nd) rather than the form of the request, so a spec like[[10, 10, 4]]was silently collapsed to a regular grid. This implements resolution 1 from the issue ("honor the requested kind"), which @d-v-b endorsed there as the direction to take now.Root cause
normalize_chunks_ndnormalizes a nested sequence into a canonicalChunksTuple, which loses whether the user requested rectilinear;create_chunk_grid_metadatathen dispatches onis_regular_nd(chunks)— andis_regular_1dcounts "all chunks equal, last one smaller" as regular.The two grids behave identically at creation time but diverge under
resize: a regular grid extends the uniform pattern while a rectilinear grid appends an edge, so an append-only workload gets a different (chunk-rewriting) layout from the one it asked for, with no warning and nothing in the metadata recording the substitution.Changes
create_chunk_grid_metadatagains a keyword-onlyrequested_rectilinear: bool | None = None. When set, it decides the grid kind; whenNone, behavior is unchanged (inferred from edge values) for callers that only have a normalizedChunksTuple(auto-chunking, etc.).chunks=requests rectilinear,init_array, a nested-sequenceshards=also makes the outer chunk-grid metadata rectilinear (the grid describes the outer layout; without this, rectilinear shards with uniform-plus-tail edges would have regressed),"auto"specs keep inferring from values.test_nested_sequence_request_stays_rectilinear_when_edges_uniform: creates[[10, 10, 4]], asserts a rectilinear grid, and asserts the post-resize append touches exactly one new chunk (c/3) instead of straddlingc/2 + c/3; plus unit tests pinning the new flag's three modes and the unchanged flat/auto inference.changes/4272.bugfix.md.Verification
Local run of the issue's reproducer on Windows/CPython 3.12:
Before (matches the issue's "On main" output):
After (matches 3.2.1 semantics from the issue):
Test suites on this branch (all green):
tests/test_unified_chunk_grid.py+tests/test_metadata+tests/test_chunk_grids.py= 612 passed,tests/test_array.py= 1307 passed / 35 skipped. The new regression test fails on pristinemainand passes with this patch.Documentation
No docs page states the old collapse behavior; the changelog fragment documents the fix.
AI
Development assisted by AI (Claude), disclosed per project policy.