Skip to content

fix: make the chunk normalizer the one judge of a chunk specification - #4376

Draft
d-v-b wants to merge 2 commits into
zarr-developers:mainfrom
d-v-b:fix/single-chunk-normalizer
Draft

d-v-b wants to merge 2 commits into
zarr-developers:mainfrom
d-v-b:fix/single-chunk-normalizer

Conversation

@d-v-b

@d-v-b d-v-b commented Sep 18, 2026

Copy link
Copy Markdown
Contributor

🤖 AI text below 🤖

Follow-up to #4374 / #4375, asking whether there is a single, correct function from user input (chunks=) to a chunk grid configuration.

What was there

The parser is normalize_chunks_ndnormalize_chunks_1d, and it already handled numpy scalars (numbers.Integral) and numpy arrays (iterated, elements coerced with int()) correctly at every entry point. But it was not the only logic on the path: a separate duck-typed classifier, _is_rectilinear_chunks, ran on the raw input at three sites (AsyncArray.create, init_array, resolve_outer_and_inner_chunks) to decide whether the spec was rectilinear, so the v2 and sharding policy errors could fire. Two opinions on the same input is the shape of the bug in #4374, and they could disagree (a 0-d numpy array counted as rectilinear because it has __iter__).

Changes

  • _is_rectilinear_chunks is deleted. Each site normalizes first and asks the ChunkGrid (is_regular); stored RectilinearChunkGridMetadata passed as chunks= still counts as rectilinear even when its edges are uniform. The shard resolver sends regular and rectilinear shard specs through the same normalizer.
  • Bug fix: the legacy v2 branch of AsyncArray.create did chunks or chunk_shape, so zarr.create(chunks=np.array([5, 3]), zarr_format=2) failed with "truth value of an array is ambiguous". It now uses the is not None form the v3 branch already had.
  • 0-d numpy arrays unwrap to their scalar in both normalizers (was "len() of unsized object").
  • A non-integer scalar spec (2.0, np.float64(2.0)) raises the normalizer's own TypeError (was "object has no len()").

Tests

  • One matrix over numpy inputs (scalar, 0-d array, tuple/list of numpy ints, 1-D array, 0-d element, numpy -1) × three creation paths (create_array v3, create_array v2, legacy create v2), asserting the stored chunks are plain ints.
  • Rectilinear numpy inputs ((2, np.array([3, 3])), 2-D arrays, edges of numpy ints) produce the expected RectilinearChunkGridMetadata.
  • numpy inputs for shards=.
  • One error test for a 0-d float array.
  • The classifier's own tests are removed with it; the existing tests for the v2 and sharding policy errors (test_mixed_chunks_gates_are_order_independent, test_pipeline_sharding_rejects_rectilinear_chunks_with_shards) still pass against the normalized-grid check.

Not changed: True is still accepted as chunk size 1 (bool is Integral); flagging rather than deciding that here.

🤖 Generated with Claude Code

Chunk specifications were parsed by `normalize_chunks_nd`, but a separate
duck-typed classifier, `_is_rectilinear_chunks`, ran on the raw input first
at three sites to decide whether the spec was rectilinear. Two opinions on
the same input is the shape of the bug in zarr-developers#4374, and they could disagree:
a 0-d numpy array counted as rectilinear because it has `__iter__`.

The classifier is gone. Each site normalizes first and asks the resulting
`ChunkGrid` (`is_regular`); stored rectilinear metadata passed as
`chunks=` counts as rectilinear even when its edges are uniform. The shard
resolver sends regular and rectilinear shard specs through the same
normalizer.

Also fixed on the way:

- The legacy v2 branch of `AsyncArray.create` tested `chunks or
  chunk_shape`, so `zarr.create(chunks=np.array([...]), zarr_format=2)`
  failed with "truth value of an array is ambiguous". It now uses the
  `is not None` form the v3 branch already had.
- 0-d numpy arrays unwrap to their scalar in both normalizers instead of
  failing with "len() of unsized object".
- A non-integer scalar spec (`2.0`, `np.float64`) raises the normalizer's
  own TypeError instead of "object has no len()".

Assisted-by: ClaudeCode:claude-fable-5-1
Assisted-by: ClaudeCode:claude-fable-5-1
@github-actions github-actions Bot added needs release notes Automatically applied to PRs which haven't added release notes and removed needs release notes Automatically applied to PRs which haven't added release notes labels Sep 18, 2026
@read-the-docs-community

read-the-docs-community Bot commented Sep 18, 2026

Copy link
Copy Markdown

@read-the-docs-community

read-the-docs-community Bot commented Sep 18, 2026

Copy link
Copy Markdown

@codecov

codecov Bot commented Sep 18, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.22%. Comparing base (ba883a5) to head (9d0f413).
⚠️ Report is 19 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4376      +/-   ##
==========================================
- Coverage   94.34%   94.22%   -0.12%     
==========================================
  Files          92       92              
  Lines       12935    12932       -3     
==========================================
- Hits        12203    12185      -18     
- Misses        732      747      +15     
Files with missing lines Coverage Δ
src/zarr/core/array.py 98.08% <100.00%> (+<0.01%) ⬆️
src/zarr/core/chunk_grids.py 96.70% <100.00%> (-0.08%) ⬇️

... and 15 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant