From 9bfe855a6cfd6985073fd5e4e5b5be027628eca8 Mon Sep 17 00:00:00 2001 From: Davis Bennett Date: Tue, 14 Jul 2026 13:25:16 +0200 Subject: [PATCH 1/9] fix: byte-order handling for structured dtypes in the bytes codec (#220) * 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 #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 --- changes/4141.bugfix.md | 1 + src/zarr/codecs/bytes.py | 33 +++++++++++----- tests/test_codecs/test_bytes.py | 69 ++++++++++++++++++++++++++------- 3 files changed, 80 insertions(+), 23 deletions(-) create mode 100644 changes/4141.bugfix.md diff --git a/changes/4141.bugfix.md b/changes/4141.bugfix.md new file mode 100644 index 0000000000..6a132da3f5 --- /dev/null +++ b/changes/4141.bugfix.md @@ -0,0 +1 @@ +Fix silent byte-order corruption for structured dtypes with the `bytes` codec: multi-byte fields are now byte-swapped to the codec's configured `endian` on write and decoded honoring it on read, so non-native-endian structured data (e.g. big-endian fields, as produced by virtual references to external data) round-trips correctly. diff --git a/src/zarr/codecs/bytes.py b/src/zarr/codecs/bytes.py index 240c077627..fae762fd08 100644 --- a/src/zarr/codecs/bytes.py +++ b/src/zarr/codecs/bytes.py @@ -100,14 +100,28 @@ def _decode_sync( chunk_spec: ArraySpec, ) -> NDBuffer: endian_str = self.endian + dtype = chunk_spec.dtype.to_native_dtype() + # The byte order of the stored data is set by this codec's `endian` + # configuration; the byte order of the decoded array is set by the array's + # data type. The two are independent: the raw bytes are viewed with a dtype + # in the stored byte order, then converted to the declared dtype if needed. if isinstance(chunk_spec.dtype, HasEndianness): - dtype = replace(chunk_spec.dtype, endianness=endian_str).to_native_dtype() # type: ignore[call-arg] + view_dtype = replace(chunk_spec.dtype, endianness=endian_str).to_native_dtype() # type: ignore[call-arg] + elif isinstance(chunk_spec.dtype, Struct) and endian_str is not None: + # Per the struct data type spec, all multi-byte fields are stored in the + # byte order configured on this codec. + view_dtype = dtype.newbyteorder(endian_str) else: - dtype = chunk_spec.dtype.to_native_dtype() + view_dtype = dtype as_array_like = chunk_bytes.as_array_like() chunk_array = chunk_spec.prototype.nd_buffer.from_ndarray_like( - as_array_like.view(dtype=dtype) # type: ignore[attr-defined] + as_array_like.view(dtype=view_dtype) # type: ignore[attr-defined] ) + if view_dtype != dtype: + # This byte-swapping conversion copies the chunk. The dtype inequality + # guard keeps the common case, where the stored and declared byte orders + # already match, on the zero-copy view path above. + chunk_array = chunk_array.astype(dtype) # ensure correct chunk shape if chunk_array.shape != chunk_spec.shape: @@ -129,13 +143,14 @@ def _encode_sync( chunk_spec: ArraySpec, ) -> Buffer | None: assert isinstance(chunk_array, NDBuffer) - if ( - chunk_array.dtype.itemsize > 1 - and self.endian is not None - and self.endian != chunk_array.byteorder - ): + if chunk_array.dtype.itemsize > 1 and self.endian is not None: + # Compare full dtypes rather than the top-level byteorder: numpy reports + # byteorder '|' for structured dtypes even when their fields are + # byte-order-sensitive, so newbyteorder is the only reliable way to + # detect (and normalize) a byte-order mismatch. new_dtype = chunk_array.dtype.newbyteorder(self.endian) - chunk_array = chunk_array.astype(new_dtype) + if new_dtype != chunk_array.dtype: + chunk_array = chunk_array.astype(new_dtype) nd_array = chunk_array.as_ndarray_like() # Flatten the nd-array (only copy if needed) and reinterpret as bytes diff --git a/tests/test_codecs/test_bytes.py b/tests/test_codecs/test_bytes.py index 03dd0b40c6..ead778f526 100644 --- a/tests/test_codecs/test_bytes.py +++ b/tests/test_codecs/test_bytes.py @@ -33,29 +33,50 @@ @pytest.mark.parametrize("store", ["local", "memory"], indirect=["store"]) -@pytest.mark.parametrize("input_dtype", [">u2", "u2", + "f4"), ("mask", ">i4")], + [("flux", "u2", " None: """ The `bytes` codec stores multi-byte data in the byte order configured on the codec, regardless of the input array's byte order, and reads it back to the - original values. The input-dtype/store-endian cross-product exercises the - encode-side byteswap (input byte order != store byte order) and the no-op - case alike. Compression is disabled so the stored chunk is the codec's raw - output and its byte layout can be asserted directly. + original values. For structured dtypes this applies to every multi-byte + field, per the `struct` data type spec; the struct cases guard against the + endianness bugs from + https://github.com/zarr-developers/zarr-python/issues/4141, where the + encode path never byte-swapped struct fields (numpy reports byteorder '|' + for void dtypes) and the decode path ignored the codec's endian entirely. + The input-dtype/store-endian cross-product exercises the encode-side + byteswap (input byte order != store byte order) and the no-op case alike. + Compression is disabled so the stored chunk is the codec's raw output and + its byte layout can be asserted directly. """ - data = np.arange(0, 256, dtype=input_dtype).reshape((16, 16)) + dtype = np.dtype(input_dtype) + if dtype.fields is None: + data = np.arange(0, 256, dtype=dtype).reshape((16, 16)) + else: + data = np.zeros((16, 16), dtype=dtype) + data["flux"] = np.arange(0, 256).reshape((16, 16)) + data["mask"] = np.arange(256, 512).reshape((16, 16)) path = "endian" spath = StorePath(store, path) a = await zarr.api.asynchronous.create_array( spath, shape=data.shape, chunks=(16, 16), - dtype="uint16", + dtype=dtype, fill_value=0, compressors=None, serializer=BytesCodec(endian=store_endian), @@ -66,8 +87,7 @@ async def test_endian( # The stored chunk is laid out in the byte order configured on the codec. stored = await store.get(f"{path}/c/0/0", prototype=default_buffer_prototype()) assert stored is not None - expected_dtype = ">u2" if store_endian == "big" else " None: assert isinstance(BytesCodec(), SupportsSyncCodec) -def test_bytes_codec_sync_roundtrip() -> None: - codec = BytesCodec() - arr = np.arange(100, dtype="float64") +@pytest.mark.parametrize("endian", ENDIAN) +@pytest.mark.parametrize( + "native_dtype", + [np.dtype("float64"), np.dtype(">u2"), np.dtype([("a", ">f4"), ("b", " None: + """ + The synchronous encode/decode path round-trips data, and the two byte + orders involved are independent: the codec's `endian` configuration governs + only the stored byte layout (every multi-byte value, including struct + fields, is laid out in the codec's byte order regardless of the input + array's byte order), while the decoded buffer's byte order is governed by + the array's data type regardless of the codec's. The mixed-endian struct + case pins that per-field byte order of the in-memory dtype survives a + roundtrip through a single stored byte order. + """ + if native_dtype.fields is None: + arr = np.arange(100, dtype=native_dtype) + else: + arr = np.array([(1.5, 2), (3.5, 4), (5.5, 6), (7.5, 8)], dtype=native_dtype) zdtype = get_data_type_from_native_dtype(arr.dtype) spec = ArraySpec( shape=arr.shape, @@ -91,11 +129,14 @@ def test_bytes_codec_sync_roundtrip() -> None: ) nd_buf: NDBuffer = default_buffer_prototype().nd_buffer.from_numpy_array(arr) - codec = codec.evolve_from_array_spec(spec) + codec = BytesCodec(endian=endian).evolve_from_array_spec(spec) encoded = codec._encode_sync(nd_buf, spec) assert encoded is not None + assert encoded.to_bytes() == arr.astype(native_dtype.newbyteorder(endian)).tobytes() + decoded = codec._decode_sync(encoded, spec) + assert decoded.dtype == zdtype.to_native_dtype() np.testing.assert_array_equal(arr, decoded.as_numpy_array()) From 6a83a11fe6f0d069d003c87b82077dc04e113abb Mon Sep 17 00:00:00 2001 From: Davis Vann Bennett Date: Wed, 22 Jul 2026 15:17:32 +0200 Subject: [PATCH 2/9] fix: informative errors for None in chunk specifications 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 #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 --- src/zarr/core/chunk_grids.py | 13 +++++++-- tests/test_chunk_grids.py | 55 ++++++++++++++++++++++++++++++++++++ 2 files changed, 65 insertions(+), 3 deletions(-) diff --git a/src/zarr/core/chunk_grids.py b/src/zarr/core/chunk_grids.py index 2cb9762775..abfe1e1b81 100644 --- a/src/zarr/core/chunk_grids.py +++ b/src/zarr/core/chunk_grids.py @@ -718,17 +718,23 @@ def _guess_regular_chunks( def normalize_chunks_1d( - chunks: int | Iterable[object], span: int + chunks: int | Iterable[object] | None, span: int ) -> np.ndarray[tuple[int], np.dtype[np.int64]]: """ Normalize a one-dimensional chunk specification into a 1D int64 array of chunk sizes that cover the span. - `-1` means "one chunk covering the entire span." + `-1` means "one chunk covering the entire span." `None` is rejected with + an informative error directing the user to `-1`. For an integer chunk size, all chunks are uniform — the last chunk may overhang the span. The actual data extent of each chunk is determined by the chunk grid at runtime, not by this function. """ + if chunks is None: + raise ValueError( + "None is not a valid chunk size for a dimension. " + "Use -1 for a single chunk covering the full extent of an axis." + ) if chunks == -1: return np.array([span], dtype=np.int64) if isinstance(chunks, int): @@ -779,7 +785,8 @@ def normalize_chunks_nd( """ if chunks is None or chunks is True: raise ValueError( - f'{chunks!r} is not a valid chunk input. Use chunks=None or chunks="auto" from the top-level API for auto-chunking, or pass an int / tuple of ints.' + f'{chunks!r} is not a valid chunk input. Use chunks="auto" or omit the chunks ' + "argument for automatic chunking, or pass an int / tuple of ints." ) # handle no chunking diff --git a/tests/test_chunk_grids.py b/tests/test_chunk_grids.py index b730a43901..ba2f74b2a2 100644 --- a/tests/test_chunk_grids.py +++ b/tests/test_chunk_grids.py @@ -3,6 +3,7 @@ import numpy as np import pytest +import zarr from tests.conftest import Expect, ExpectFail from zarr.core.chunk_grids import ( ChunkLayout, @@ -142,6 +143,12 @@ def test_chunk_layout_nested() -> None: id="negative-uniform", msg="Chunk size must be positive", ), + ExpectFail( + input=(None, 100), + exception=ValueError, + id="none-chunk-size", + msg="None is not a valid chunk size for a dimension", + ), ExpectFail(input=([], 100), exception=ValueError, id="empty-list", msg="must not be empty"), ExpectFail( input=([10, -1, 10], 100), @@ -206,6 +213,14 @@ def test_normalize_chunks_1d_errors(case: ExpectFail[tuple[Any, int]]) -> None: id="true", msg="True is not a valid chunk input", ), + # A per-dimension `None` (dask-style "full extent") is rejected with a + # pointer to the `-1` sentinel. + ExpectFail( + input=((None, 5), (100, 100)), + exception=ValueError, + id="per-dimension-none", + msg="Use -1 for a single chunk covering the full extent of an axis", + ), ExpectFail(input=("foo", (100,)), exception=ValueError, id="string", msg="dimensions"), ExpectFail( input=((100, 10), (100,)), exception=ValueError, id="too-many-dims", msg="dimensions" @@ -253,3 +268,43 @@ def test_normalize_chunks_1d_returns_int64_array( assert result.dtype == np.int64 assert result.ndim == 1 assert result.tolist() == case.output + + +@pytest.mark.parametrize( + ("chunks", "expected"), + [ + (-1, (10, 10)), + ((-1, -1), (10, 10)), + ((-1, 5), (10, 5)), + ("auto", (10, 10)), + ], + ids=["scalar-minus-one", "tuple-minus-one", "mixed-minus-one", "auto"], +) +def test_create_array_valid_chunk_forms(chunks: Any, expected: tuple[int, ...]) -> None: + """Valid chunk specifications produce arrays with the expected chunk shape.""" + arr = zarr.create_array(store={}, shape=(10, 10), dtype="i4", chunks=chunks) + assert arr.chunks == expected + + +def test_create_rejects_per_dimension_none_chunk() -> None: + """A per-dimension None in `chunks` via `zarr.create` raises an informative error.""" + with pytest.raises( + ValueError, match="Use -1 for a single chunk covering the full extent of an axis" + ): + zarr.create(store={}, shape=(10, 10), dtype="i4", chunks=(None, 5)) # type: ignore[arg-type] + + +def test_create_array_rejects_per_dimension_none_chunk() -> None: + """A per-dimension None in `chunks` via `zarr.create_array` raises an informative error.""" + with pytest.raises( + ValueError, match="Use -1 for a single chunk covering the full extent of an axis" + ): + zarr.create_array(store={}, shape=(10, 10), dtype="i4", chunks=(None, 5)) # type: ignore[arg-type] + + +def test_create_array_rejects_chunks_none() -> None: + """`chunks=None` via `zarr.create_array` points the user at `chunks="auto"`.""" + with pytest.raises( + ValueError, match='Use chunks="auto" or omit the chunks argument for automatic chunking' + ): + zarr.create_array(store={}, shape=(10, 10), dtype="i4", chunks=None) # type: ignore[arg-type] From 4d57b1b5246ce764296ed2d0f3c8b2636808fd56 Mon Sep 17 00:00:00 2001 From: Davis Vann Bennett Date: Wed, 22 Jul 2026 15:52:51 +0200 Subject: [PATCH 3/9] refactor: type chunk normalizers as object and narrow internally MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 #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 --- src/zarr/core/chunk_grids.py | 99 +++++++++++++++++++++++------------- tests/test_chunk_grids.py | 80 ++++++++++++++++++++++++++++- 2 files changed, 144 insertions(+), 35 deletions(-) diff --git a/src/zarr/core/chunk_grids.py b/src/zarr/core/chunk_grids.py index abfe1e1b81..d3d56474f8 100644 --- a/src/zarr/core/chunk_grids.py +++ b/src/zarr/core/chunk_grids.py @@ -6,6 +6,7 @@ import numbers import operator import warnings +from collections.abc import Iterable from dataclasses import dataclass, field from functools import reduce from typing import ( @@ -31,7 +32,7 @@ from zarr.errors import ZarrUserWarning if TYPE_CHECKING: - from collections.abc import Iterable, Iterator, Sequence + from collections.abc import Iterator, Sequence from zarr.core.array import ShardsLike from zarr.core.metadata import ArrayMetadata @@ -717,15 +718,16 @@ def _guess_regular_chunks( return tuple(int(x) for x in chunks) -def normalize_chunks_1d( - chunks: int | Iterable[object] | None, span: int -) -> np.ndarray[tuple[int], np.dtype[np.int64]]: +def normalize_chunks_1d(chunks: object, span: int) -> np.ndarray[tuple[int], np.dtype[np.int64]]: """ Normalize a one-dimensional chunk specification into a 1D int64 array of chunk sizes that cover the span. - `-1` means "one chunk covering the entire span." `None` is rejected with - an informative error directing the user to `-1`. + Accepts `object` and narrows internally: `None` and `bool` are rejected + with informative errors (`-1` is the sentinel for "one chunk covering the + entire span"), strings and other non-iterables are rejected, and any other + iterable (including a generator) is materialized as an explicit list of + chunk sizes. For an integer chunk size, all chunks are uniform — the last chunk may overhang the span. The actual data extent of each chunk is determined by the chunk grid at runtime, not by this function. @@ -735,6 +737,14 @@ def normalize_chunks_1d( "None is not a valid chunk size for a dimension. " "Use -1 for a single chunk covering the full extent of an axis." ) + # bool is a subclass of int, so without this guard True would silently + # pass through the integer branch below as chunk size 1. + if chunks is True or chunks is False: + raise ValueError( + f"{chunks} is not a valid chunk size for a dimension. " + "Chunk sizes must be positive integers, or -1 for a single chunk " + "covering the full extent of an axis." + ) if chunks == -1: return np.array([span], dtype=np.int64) if isinstance(chunks, int): @@ -744,39 +754,49 @@ def normalize_chunks_1d( return np.array([chunks], dtype=np.int64) n = ceildiv(span, chunks) return np.full(n, chunks, dtype=np.int64) - else: - chunk_list = list(chunks) - if not chunk_list: - raise ValueError("Chunk specification must not be empty") - non_int = [ - (idx, c) for idx, c in enumerate(chunk_list) if not isinstance(c, numbers.Integral) - ] - if non_int: - non_int_idxs, non_int_vals = [*zip(*non_int, strict=False)] - raise TypeError( - f"Each chunk size must be an integer; got non-integer element(s) {non_int_vals!r} " - f"at indices {non_int_idxs!r}. Chunk sizes must be declared as a flat sequence of " - f"positive integers (e.g. [3, 3, 1])." - ) - ints: list[int] = [int(c) for c in chunk_list] # type: ignore[call-overload] - if any(c <= 0 for c in ints): - raise ValueError(f"All chunk sizes must be positive, got {ints}") - if sum(ints) != span: - raise ValueError(f"Chunk sizes {ints} do not sum to span {span}") - return np.asarray(ints, dtype=np.int64) + if isinstance(chunks, (str, bytes)): + raise TypeError( + f"{chunks!r} is not a valid chunk size for a dimension. " + "Expected an int or an iterable of ints." + ) + if not isinstance(chunks, Iterable): + raise TypeError( + f"{chunks!r} is not a valid chunk size for a dimension. " + "Expected an int or an iterable of ints." + ) + chunk_list = list(chunks) + if not chunk_list: + raise ValueError("Chunk specification must not be empty") + non_int = [(idx, c) for idx, c in enumerate(chunk_list) if not isinstance(c, numbers.Integral)] + if non_int: + non_int_idxs, non_int_vals = [*zip(*non_int, strict=False)] + raise TypeError( + f"Each chunk size must be an integer; got non-integer element(s) {non_int_vals!r} " + f"at indices {non_int_idxs!r}. Chunk sizes must be declared as a flat sequence of " + f"positive integers (e.g. [3, 3, 1])." + ) + ints: list[int] = [int(c) for c in chunk_list] + if any(c <= 0 for c in ints): + raise ValueError(f"All chunk sizes must be positive, got {ints}") + if sum(ints) != span: + raise ValueError(f"Chunk sizes {ints} do not sum to span {span}") + return np.asarray(ints, dtype=np.int64) def normalize_chunks_nd( - chunks: Any, + chunks: object, shape: tuple[int, ...], ) -> ChunksTuple: """ Normalize a chunk specification into a `ChunksTuple`. This is a mechanical transformation — no heuristics, no guessing. - Handles `False` ("all data in one chunk"), scalar ints, `-1` sentinels (one chunk - per dimension covering the full span), and explicit per-dimension lists - of chunk sizes (regular or rectilinear). + Accepts `object` and narrows internally: `False` ("all data in one chunk"), + scalar ints, `-1` sentinels (one chunk per dimension covering the full + span), and per-dimension iterables of chunk sizes (regular or + rectilinear). Any non-string iterable — including a generator — is + materialized before use; strings and non-iterables are rejected with + informative errors. For auto-chunking, use `guess_chunks` which returns a `ChunksTuple` directly. `chunks=None` and `chunks=True` are rejected @@ -795,16 +815,27 @@ def normalize_chunks_nd( # handle 1D convenience form. bool is excluded above so this only catches actual ints. if isinstance(chunks, numbers.Integral): - chunks = tuple(int(chunks) for _ in shape) + chunks_tuple: tuple[Any, ...] = tuple(int(chunks) for _ in shape) + elif isinstance(chunks, (str, bytes)): + raise TypeError( + f"{chunks!r} is not a valid chunk input. Expected an int or an iterable of ints." + ) + elif isinstance(chunks, Iterable): + # materialize before use so generators are supported and len() is safe + chunks_tuple = tuple(chunks) + else: + raise TypeError( + f"{chunks!r} is not a valid chunk input. Expected an int or an iterable of ints." + ) # handle bad dimensionality - if len(chunks) != len(shape): + if len(chunks_tuple) != len(shape): raise ValueError( - f"chunks has {len(chunks)} dimensions but shape has {len(shape)} dimensions" + f"chunks has {len(chunks_tuple)} dimensions but shape has {len(shape)} dimensions" ) return ChunksTuple( - tuple(normalize_chunks_1d(c, span=s) for c, s in zip(chunks, shape, strict=True)) + tuple(normalize_chunks_1d(c, span=s) for c, s in zip(chunks_tuple, shape, strict=True)) ) diff --git a/tests/test_chunk_grids.py b/tests/test_chunk_grids.py index ba2f74b2a2..9910c30f4a 100644 --- a/tests/test_chunk_grids.py +++ b/tests/test_chunk_grids.py @@ -1,3 +1,4 @@ +from collections.abc import Callable from typing import Any import numpy as np @@ -149,6 +150,29 @@ def test_chunk_layout_nested() -> None: id="none-chunk-size", msg="None is not a valid chunk size for a dimension", ), + # bool is a subclass of int; without an explicit guard, True would + # silently pass through the int branch as chunk size 1. + ExpectFail( + input=(True, 100), + exception=ValueError, + id="bool-chunk-size", + msg="True is not a valid chunk size for a dimension", + ), + # Non-iterable, non-int values must get an informative error rather + # than a bare crash from list(). + ExpectFail( + input=(2.5, 100), + exception=TypeError, + id="float-chunk-size", + msg="Expected an int or an iterable of ints", + ), + # Strings are iterable but never a valid chunk size. + ExpectFail( + input=("5", 100), + exception=TypeError, + id="string-chunk-size", + msg="Expected an int or an iterable of ints", + ), ExpectFail(input=([], 100), exception=ValueError, id="empty-list", msg="must not be empty"), ExpectFail( input=([10, -1, 10], 100), @@ -221,7 +245,36 @@ def test_normalize_chunks_1d_errors(case: ExpectFail[tuple[Any, int]]) -> None: id="per-dimension-none", msg="Use -1 for a single chunk covering the full extent of an axis", ), - ExpectFail(input=("foo", (100,)), exception=ValueError, id="string", msg="dimensions"), + # A per-dimension `True` must not silently become chunk size 1. + ExpectFail( + input=((True, 5), (100, 100)), + exception=ValueError, + id="per-dimension-true", + msg="True is not a valid chunk size for a dimension", + ), + # A non-iterable per-dimension value gets an informative error. + ExpectFail( + input=((2.5, 5), (100, 100)), + exception=TypeError, + id="per-dimension-float", + msg="Expected an int or an iterable of ints", + ), + # Strings are iterable but rejected outright. Note that `chunks="auto"` + # is intercepted by the callers before normalization, so a string here + # always means invalid input. + ExpectFail( + input=("foo", (100,)), + exception=TypeError, + id="string", + msg="not a valid chunk input", + ), + # A non-iterable whole argument gets an informative error. + ExpectFail( + input=(2.5, (100,)), + exception=TypeError, + id="non-iterable", + msg="Expected an int or an iterable of ints", + ), ExpectFail( input=((100, 10), (100,)), exception=ValueError, id="too-many-dims", msg="dimensions" ), @@ -286,6 +339,31 @@ def test_create_array_valid_chunk_forms(chunks: Any, expected: tuple[int, ...]) assert arr.chunks == expected +@pytest.mark.parametrize( + ("make_chunks", "shape", "expected"), + [ + # whole-argument generator of per-dimension sizes + (lambda: (10 for _ in range(2)), (100, 10), ((10,) * 10, (10,))), + # rectilinear spec with a generator as one dimension's sizes + (lambda: ((c for c in (60, 40)), [5, 5]), (100, 10), ((60, 40), (5, 5))), + ], + ids=["whole-argument-generator", "per-dimension-generator"], +) +def test_normalize_chunks_nd_accepts_generators( + make_chunks: Callable[[], Any], + shape: tuple[int, ...], + expected: tuple[tuple[int, ...], ...], +) -> None: + """Generator inputs are materialized and normalized like any other iterable.""" + _assert_chunks_equal(normalize_chunks_nd(make_chunks(), shape), expected) + + +def test_create_rejects_per_dimension_true_chunk() -> None: + """A per-dimension True in `chunks` via `zarr.create` must not become chunk size 1.""" + with pytest.raises(ValueError, match="True is not a valid chunk size for a dimension"): + zarr.create(store={}, shape=(10, 10), dtype="i4", chunks=(True, 5)) + + def test_create_rejects_per_dimension_none_chunk() -> None: """A per-dimension None in `chunks` via `zarr.create` raises an informative error.""" with pytest.raises( From 27d8b3b22cb1827a758e6e2b4737c5c046b19947 Mon Sep 17 00:00:00 2001 From: Davis Vann Bennett Date: Wed, 22 Jul 2026 16:13:09 +0200 Subject: [PATCH 4/9] test: update stale error-message assertion for float chunk sizes 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 --- tests/test_api.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_api.py b/tests/test_api.py index 1b4414ae63..f074a14bbf 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -79,7 +79,7 @@ def test_create(memory_store: Store) -> None: z = create(shape=(400.5, 100), store=store, overwrite=True) # type: ignore[arg-type] # create array with float chunk shape - with pytest.raises(TypeError, match="'float' object is not iterable"): + with pytest.raises(TypeError, match="is not a valid chunk size for a dimension"): z = create(shape=(400, 100), chunks=(16, 16.5), store=store, overwrite=True) # type: ignore[arg-type] From 45a1b2c13ec84cffd458755e797aaee4332b9536 Mon Sep 17 00:00:00 2001 From: Davis Vann Bennett Date: Wed, 22 Jul 2026 16:14:57 +0200 Subject: [PATCH 5/9] refactor: collapse duplicated type-rejection branches in chunk normalizers 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 --- src/zarr/core/chunk_grids.py | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/src/zarr/core/chunk_grids.py b/src/zarr/core/chunk_grids.py index d3d56474f8..7e721cfa3d 100644 --- a/src/zarr/core/chunk_grids.py +++ b/src/zarr/core/chunk_grids.py @@ -754,12 +754,8 @@ def normalize_chunks_1d(chunks: object, span: int) -> np.ndarray[tuple[int], np. return np.array([chunks], dtype=np.int64) n = ceildiv(span, chunks) return np.full(n, chunks, dtype=np.int64) - if isinstance(chunks, (str, bytes)): - raise TypeError( - f"{chunks!r} is not a valid chunk size for a dimension. " - "Expected an int or an iterable of ints." - ) - if not isinstance(chunks, Iterable): + # str/bytes are iterable but never a valid chunk specification + if isinstance(chunks, (str, bytes)) or not isinstance(chunks, Iterable): raise TypeError( f"{chunks!r} is not a valid chunk size for a dimension. " "Expected an int or an iterable of ints." @@ -816,12 +812,9 @@ def normalize_chunks_nd( # handle 1D convenience form. bool is excluded above so this only catches actual ints. if isinstance(chunks, numbers.Integral): chunks_tuple: tuple[Any, ...] = tuple(int(chunks) for _ in shape) - elif isinstance(chunks, (str, bytes)): - raise TypeError( - f"{chunks!r} is not a valid chunk input. Expected an int or an iterable of ints." - ) - elif isinstance(chunks, Iterable): - # materialize before use so generators are supported and len() is safe + elif isinstance(chunks, Iterable) and not isinstance(chunks, (str, bytes)): + # materialize before use so generators are supported and len() is safe; + # str/bytes are iterable but never a valid chunk specification chunks_tuple = tuple(chunks) else: raise TypeError( From 4203be6c6e2dabb1c4e4002e2d40c31198c239ca Mon Sep 17 00:00:00 2001 From: Davis Vann Bennett Date: Wed, 22 Jul 2026 16:19:10 +0200 Subject: [PATCH 6/9] refactor: move auto-chunking guidance out of the chunk normalizer 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 --- src/zarr/core/array.py | 12 +++++++++++- src/zarr/core/chunk_grids.py | 4 ++-- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/src/zarr/core/array.py b/src/zarr/core/array.py index f75ef72415..49d5bcf4d3 100644 --- a/src/zarr/core/array.py +++ b/src/zarr/core/array.py @@ -4461,7 +4461,17 @@ async def init_array( "chunks=(inner_size, ...), shards=[[shard_sizes], ...]" ) - # Normalize the user's chunks into canonical ChunksTuple form + # Normalize the user's chunks into canonical ChunksTuple form. + # Auto-chunking is an API-level concept, so the guidance toward it is + # raised here rather than in the mechanical normalizer. Validate through + # an object-typed view: None/True are outside ChunksLike but reachable + # from untyped callers. + chunks_input: object = chunks + if chunks_input is None or chunks_input is True: + raise ValueError( + f'{chunks!r} is not a valid chunk input. Use chunks="auto" or omit the chunks ' + "argument for automatic chunking, or pass an int / tuple of ints." + ) if chunks == "auto": max_bytes = None if shards is None else SHARDED_INNER_CHUNK_MAX_BYTES diff --git a/src/zarr/core/chunk_grids.py b/src/zarr/core/chunk_grids.py index 7e721cfa3d..fc28f7526c 100644 --- a/src/zarr/core/chunk_grids.py +++ b/src/zarr/core/chunk_grids.py @@ -801,8 +801,8 @@ def normalize_chunks_nd( """ if chunks is None or chunks is True: raise ValueError( - f'{chunks!r} is not a valid chunk input. Use chunks="auto" or omit the chunks ' - "argument for automatic chunking, or pass an int / tuple of ints." + f"{chunks!r} is not a valid chunk input. " + "Expected an int, an iterable of ints, or False." ) # handle no chunking From f053f454c528d0ffff00226b75ea60c163016fb2 Mon Sep 17 00:00:00 2001 From: Davis Vann Bennett Date: Wed, 22 Jul 2026 23:05:45 +0200 Subject: [PATCH 7/9] docs: add 3.3.0 release note for chunk normalization hardening Assisted-by: ClaudeCode:claude-fable-5 --- docs/release-notes.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/docs/release-notes.md b/docs/release-notes.md index 3fd8a5f360..d3e24f5d67 100644 --- a/docs/release-notes.md +++ b/docs/release-notes.md @@ -37,6 +37,13 @@ the array's shape), and `None` as a per-dimension chunk size. These all now raise informative errors. Also fix chunk handling for 0-length array dimensions, and add explicit rejection of 0-length chunks. ([#3899](https://github.com/zarr-developers/zarr-python/issues/3899)) +- Further hardened chunk normalization: a per-dimension `None` chunk size now + raises an informative `ValueError` directing users to the `-1` sentinel + (previously an uninformative `TypeError`), per-dimension boolean chunk sizes + are rejected instead of `True` silently producing size-1 chunks, and strings + and other non-iterable chunk inputs raise informative `TypeError`s instead of + bare crashes. Generator inputs to chunk normalization are now materialized + and accepted. ([#4177](https://github.com/zarr-developers/zarr-python/issues/4177)) - Handle missing consolidated metadata in leaf Group nodes. ([#3954](https://github.com/zarr-developers/zarr-python/issues/3954)) - Corrected the JSON type definitions for the `numpy.datetime64` and `numpy.timedelta64` data types in Zarr V3 metadata: the `configuration` object From a9c59b668f99a459a16815a56d87b154bfb51635 Mon Sep 17 00:00:00 2001 From: Davis Bennett Date: Wed, 22 Jul 2026 23:28:59 +0200 Subject: [PATCH 8/9] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- src/zarr/core/array.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/zarr/core/array.py b/src/zarr/core/array.py index 49d5bcf4d3..8aba35454e 100644 --- a/src/zarr/core/array.py +++ b/src/zarr/core/array.py @@ -4470,7 +4470,7 @@ async def init_array( if chunks_input is None or chunks_input is True: raise ValueError( f'{chunks!r} is not a valid chunk input. Use chunks="auto" or omit the chunks ' - "argument for automatic chunking, or pass an int / tuple of ints." + "argument for automatic chunking, or pass an int / iterable of ints." ) if chunks == "auto": From 847040482b164c21bbf59f97dee48991661b5626 Mon Sep 17 00:00:00 2001 From: Davis Vann Bennett Date: Wed, 22 Jul 2026 23:32:49 +0200 Subject: [PATCH 9/9] test: cover False, list, and numpy-int chunk forms in create_array Documents that chunks=False (one whole-array chunk, v2 compat), list specs, and numpy integer scalars are valid create_array inputs, in response to review discussion on the init_array None/True guard. Assisted-by: ClaudeCode:claude-fable-5 --- tests/test_chunk_grids.py | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/tests/test_chunk_grids.py b/tests/test_chunk_grids.py index 9910c30f4a..b03ad99e2e 100644 --- a/tests/test_chunk_grids.py +++ b/tests/test_chunk_grids.py @@ -330,8 +330,19 @@ def test_normalize_chunks_1d_returns_int64_array( ((-1, -1), (10, 10)), ((-1, 5), (10, 5)), ("auto", (10, 10)), + (False, (10, 10)), + ([5, 5], (5, 5)), + (np.int64(5), (5, 5)), + ], + ids=[ + "scalar-minus-one", + "tuple-minus-one", + "mixed-minus-one", + "auto", + "false-single-chunk", + "list", + "numpy-int", ], - ids=["scalar-minus-one", "tuple-minus-one", "mixed-minus-one", "auto"], ) def test_create_array_valid_chunk_forms(chunks: Any, expected: tuple[int, ...]) -> None: """Valid chunk specifications produce arrays with the expected chunk shape."""