fix: informative errors for None in chunk specifications#4177
Open
d-v-b wants to merge 10 commits into
Open
Conversation
* fix: byte-order handling for structured dtypes in the bytes codec The bytes codec neither byte-swapped structured-dtype fields to its configured endian on encode (numpy reports byteorder '|' for void dtypes, so the top-level byteorder comparison never detected a mismatch) nor honored its endian when decoding, silently corrupting any structured data whose field byte order differed from the stored one (e.g. virtual references to external big-endian data). Encode now detects byte-order mismatches by comparing full dtypes via newbyteorder, and decode reinterprets raw bytes in the stored byte order before converting to the data type's declared byte order, so the stored layout (codec state) and the in-memory layout (array data type) are independent. Closes zarr-developers#4141 Assisted-by: ClaudeCode:claude-fable-5 * test: fold structured byte-order cases into existing bytes codec tests Extend test_endian's parametrization with structured dtypes and test_bytes_codec_sync_roundtrip with endian/dtype parametrization plus stored-layout and decoded-dtype assertions, instead of adding parallel test functions for the same properties. Assisted-by: ClaudeCode:claude-fable-5 * refactor: rename stored_dtype to view_dtype in BytesCodec decode The variable is the dtype used to view the raw chunk bytes (byte order from the codec's endian configuration), not a property of the stored data or of the returned buffer, which always carries the array's declared dtype. Assisted-by: ClaudeCode:claude-fable-5 * docs: note that the decode-side byte-order conversion copies the chunk Assisted-by: ClaudeCode:claude-fable-5
Two pre-release fixes for chunk normalization error messages in 3.3.0:
- A per-dimension None chunk size (e.g. chunks=(None, 5)), which worked in
3.2.1 as "full extent for this dimension", previously crashed with an
uninformative TypeError ('NoneType' object is not iterable) from
normalize_chunks_1d. It now raises a ValueError directing the user to
the -1 sentinel, as promised by the zarr-developers#3899 release notes entry.
- zarr.create_array(..., chunks=None) raised a self-contradictory message
telling the user to pass chunks=None "from the top-level API". The
message now points at chunks="auto" or omitting the chunks argument.
Assisted-by: ClaudeCode:claude-fable-5
Per review on PR #236, normalize_chunks_1d and normalize_chunks_nd now take object and narrow with explicit isinstance/identity checks instead of growing an ad-hoc union annotation. Behavior changes: - A per-dimension bool chunk size (e.g. chunks=(True, 5)) is now rejected with an informative ValueError. Previously bool being a subclass of int let True through as a silent size-1 chunk — the exact behavior the zarr-developers#3899 release notes say was removed. - Strings/bytes and non-iterable values (e.g. chunks=2.5 or chunks=(2.5, 5)) now raise informative TypeErrors instead of bare crashes (list(2.5), len(generator)) or a misleading dimension-count error for whole-argument strings. - Generator inputs to normalize_chunks_nd are now materialized and accepted, both as the whole argument and as a per-dimension size list in rectilinear specs, consistent with numpy-style APIs. The type: ignore[call-overload] on int(c) is no longer needed after proper narrowing. Assisted-by: ClaudeCode:claude-fable-5
tests/test_api.py::test_create pinned the old bare TypeError message
('float' object is not iterable) that the chunk-normalizer refactor
deliberately replaced with an informative one. Match the new message.
Assisted-by: ClaudeCode:claude-fable-5
…izers The str/bytes rejection and the non-iterable rejection raised identical errors from separate branches in both normalize_chunks_1d and normalize_chunks_nd. Fold each pair into a single condition; str/bytes only need naming because they are iterable. Assisted-by: ClaudeCode:claude-fable-5
normalize_chunks_nd is a mechanical routine and should not refer to chunks="auto", which it does not itself accept. Its None/True rejection now states only what the normalizer expects; the guidance pointing users at chunks="auto" (or omitting the argument) is raised in init_array, the layer where auto-chunking is actually interpreted. Assisted-by: ClaudeCode:claude-fable-5
github-actions
Bot
added
the
needs release notes
d-v-b
changed the title
d-v-b
marked this pull request as ready for review
July 22, 2026 14:39
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.
Summary
Passing
Noneas achunksparameter results in an uninformativeTypeError, when it should raise an informativeValueErrorinstead. This PR makes that change, along with some improvements to the signature of the chunk normalization routines. These routines are now shaped like regular normalization functions, with a wide input type (object) and a narrow output type.These changes were written by claude.
See d-v-b#236 for the original claude-authored PR.
Author attestation
TODO
docs/user-guide/*.mdchanges/