From 08e82b5e9575f768e80ed6f8cd9125a60abddba2 Mon Sep 17 00:00:00 2001 From: Davis Vann Bennett Date: Mon, 13 Jul 2026 09:18:45 +0200 Subject: [PATCH 1/4] 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 --- changes/4141.bugfix.md | 1 + src/zarr/codecs/bytes.py | 30 +++++++---- tests/test_codecs/test_bytes.py | 91 +++++++++++++++++++++++++++++++++ 3 files changed, 113 insertions(+), 9 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..55d237d213 100644 --- a/src/zarr/codecs/bytes.py +++ b/src/zarr/codecs/bytes.py @@ -100,14 +100,25 @@ 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 reinterpreted 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] + stored_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. + stored_dtype = dtype.newbyteorder(endian_str) else: - dtype = chunk_spec.dtype.to_native_dtype() + stored_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=stored_dtype) # type: ignore[attr-defined] ) + if stored_dtype != dtype: + chunk_array = chunk_array.astype(dtype) # ensure correct chunk shape if chunk_array.shape != chunk_spec.shape: @@ -129,13 +140,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..8dcc01e8c3 100644 --- a/tests/test_codecs/test_bytes.py +++ b/tests/test_codecs/test_bytes.py @@ -74,6 +74,56 @@ async def test_endian( assert np.array_equal(data, readback_data) +@pytest.mark.parametrize("store", ["local", "memory"], indirect=["store"]) +@pytest.mark.parametrize("input_byteorder", [">", "<"]) +@pytest.mark.parametrize("store_endian", ["big", "little"]) +async def test_endian_structured( + store: Store, + input_byteorder: Literal[">", "<"], + store_endian: Literal["big", "little"], +) -> None: + """ + The `bytes` codec applies its configured byte order to every multi-byte + field of a structured dtype, per the `struct` data type spec: the stored + chunk is laid out with all fields in the codec's byte order regardless of + the input array's field byte order, and the data reads back to the + original values. This guards against the structured-dtype 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. + Compression is disabled so the stored chunk's byte layout can be asserted + directly. + """ + input_dtype = np.dtype([("flux", f"{input_byteorder}f4"), ("mask", f"{input_byteorder}i4")]) + data = np.array([(1.5, 2), (3.5, 4), (5.5, 6)], dtype=input_dtype) + path = "endian_structured" + spath = StorePath(store, path) + a = await zarr.api.asynchronous.create_array( + spath, + shape=data.shape, + chunks=data.shape, + dtype=input_dtype, + compressors=None, + serializer=BytesCodec(endian=store_endian), + ) + + await _AsyncArrayProxy(a)[:].set(data) + + # The stored chunk has every multi-byte field in the codec's byte order. + stored = await store.get(f"{path}/c/0", prototype=default_buffer_prototype()) + assert stored is not None + stored_byteorder = ">" if store_endian == "big" else "<" + expected_dtype = np.dtype( + [("flux", f"{stored_byteorder}f4"), ("mask", f"{stored_byteorder}i4")] + ) + assert stored.to_bytes() == data.astype(expected_dtype).tobytes() + + # ... and the data reads back to the original values. + readback_data = np.asarray(await _AsyncArrayProxy(a)[:].get()) + for field in ("flux", "mask"): + assert np.array_equal(data[field], readback_data[field]) + + def test_bytes_codec_supports_sync() -> None: assert isinstance(BytesCodec(), SupportsSyncCodec) @@ -99,6 +149,47 @@ def test_bytes_codec_sync_roundtrip() -> None: np.testing.assert_array_equal(arr, decoded.as_numpy_array()) +@pytest.mark.parametrize("endian", ENDIAN) +@pytest.mark.parametrize( + "native_dtype", + [np.dtype(">u2"), np.dtype("f4"), ("b", " None: + """ + The codec's `endian` configuration governs only the stored byte layout; + the byte order of the decoded array is governed by the array's data type. + Encoding lays out every multi-byte value (including struct fields) in the + codec's byte order regardless of the input array's byte order, and decoding + returns a buffer in the data type's declared byte order 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: + data = np.arange(4, dtype=native_dtype) + else: + data = np.array([(1.5, 2), (3.5, 4), (5.5, 6), (7.5, 8)], dtype=native_dtype) + zdtype = get_data_type_from_native_dtype(native_dtype) + spec = ArraySpec( + shape=data.shape, + dtype=zdtype, + fill_value=zdtype.cast_scalar(0), + config=ArrayConfig(order="C", write_empty_chunks=True), + prototype=default_buffer_prototype(), + ) + codec = BytesCodec(endian=endian) + + encoded = codec._encode_sync(default_buffer_prototype().nd_buffer.from_numpy_array(data), spec) + assert encoded is not None + assert encoded.to_bytes() == data.astype(native_dtype.newbyteorder(endian)).tobytes() + + decoded = codec._decode_sync(encoded, spec) + assert decoded.dtype == zdtype.to_native_dtype() + np.testing.assert_array_equal(decoded.as_numpy_array(), data) + + @pytest.mark.parametrize("endian", ENDIAN) def test_bytes_codec_accepts_all_endians(endian: EndianLiteral) -> None: """ From e2ca98b5a03a0e31c69afbf8bd7a0aadfa603cde Mon Sep 17 00:00:00 2001 From: Davis Vann Bennett Date: Mon, 13 Jul 2026 09:36:25 +0200 Subject: [PATCH 2/4] 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 --- tests/test_codecs/test_bytes.py | 152 +++++++++++--------------------- 1 file changed, 51 insertions(+), 101 deletions(-) diff --git a/tests/test_codecs/test_bytes.py b/tests/test_codecs/test_bytes.py index 8dcc01e8c3..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,128 +87,57 @@ 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 "", "<"]) -@pytest.mark.parametrize("store_endian", ["big", "little"]) -async def test_endian_structured( - store: Store, - input_byteorder: Literal[">", "<"], - store_endian: Literal["big", "little"], -) -> None: - """ - The `bytes` codec applies its configured byte order to every multi-byte - field of a structured dtype, per the `struct` data type spec: the stored - chunk is laid out with all fields in the codec's byte order regardless of - the input array's field byte order, and the data reads back to the - original values. This guards against the structured-dtype 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. - Compression is disabled so the stored chunk's byte layout can be asserted - directly. - """ - input_dtype = np.dtype([("flux", f"{input_byteorder}f4"), ("mask", f"{input_byteorder}i4")]) - data = np.array([(1.5, 2), (3.5, 4), (5.5, 6)], dtype=input_dtype) - path = "endian_structured" - spath = StorePath(store, path) - a = await zarr.api.asynchronous.create_array( - spath, - shape=data.shape, - chunks=data.shape, - dtype=input_dtype, - compressors=None, - serializer=BytesCodec(endian=store_endian), - ) - - await _AsyncArrayProxy(a)[:].set(data) - - # The stored chunk has every multi-byte field in the codec's byte order. - stored = await store.get(f"{path}/c/0", prototype=default_buffer_prototype()) - assert stored is not None - stored_byteorder = ">" if store_endian == "big" else "<" - expected_dtype = np.dtype( - [("flux", f"{stored_byteorder}f4"), ("mask", f"{stored_byteorder}i4")] - ) - assert stored.to_bytes() == data.astype(expected_dtype).tobytes() - - # ... and the data reads back to the original values. - readback_data = np.asarray(await _AsyncArrayProxy(a)[:].get()) - for field in ("flux", "mask"): - assert np.array_equal(data[field], readback_data[field]) - - def test_bytes_codec_supports_sync() -> None: assert isinstance(BytesCodec(), SupportsSyncCodec) -def test_bytes_codec_sync_roundtrip() -> None: - codec = BytesCodec() - arr = np.arange(100, dtype="float64") - zdtype = get_data_type_from_native_dtype(arr.dtype) - spec = ArraySpec( - shape=arr.shape, - dtype=zdtype, - fill_value=zdtype.cast_scalar(0), - config=ArrayConfig(order="C", write_empty_chunks=True), - prototype=default_buffer_prototype(), - ) - nd_buf: NDBuffer = default_buffer_prototype().nd_buffer.from_numpy_array(arr) - - codec = codec.evolve_from_array_spec(spec) - - encoded = codec._encode_sync(nd_buf, spec) - assert encoded is not None - decoded = codec._decode_sync(encoded, spec) - np.testing.assert_array_equal(arr, decoded.as_numpy_array()) - - @pytest.mark.parametrize("endian", ENDIAN) @pytest.mark.parametrize( "native_dtype", - [np.dtype(">u2"), np.dtype("f4"), ("b", "u2"), np.dtype([("a", ">f4"), ("b", " None: +def test_bytes_codec_sync_roundtrip(endian: EndianLiteral, native_dtype: np.dtype[Any]) -> None: """ - The codec's `endian` configuration governs only the stored byte layout; - the byte order of the decoded array is governed by the array's data type. - Encoding lays out every multi-byte value (including struct fields) in the - codec's byte order regardless of the input array's byte order, and decoding - returns a buffer in the data type's declared byte order 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. + 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: - data = np.arange(4, dtype=native_dtype) + arr = np.arange(100, dtype=native_dtype) else: - data = np.array([(1.5, 2), (3.5, 4), (5.5, 6), (7.5, 8)], dtype=native_dtype) - zdtype = get_data_type_from_native_dtype(native_dtype) + 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=data.shape, + shape=arr.shape, dtype=zdtype, fill_value=zdtype.cast_scalar(0), config=ArrayConfig(order="C", write_empty_chunks=True), prototype=default_buffer_prototype(), ) - codec = BytesCodec(endian=endian) + nd_buf: NDBuffer = default_buffer_prototype().nd_buffer.from_numpy_array(arr) - encoded = codec._encode_sync(default_buffer_prototype().nd_buffer.from_numpy_array(data), 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() == data.astype(native_dtype.newbyteorder(endian)).tobytes() + 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(decoded.as_numpy_array(), data) + np.testing.assert_array_equal(arr, decoded.as_numpy_array()) @pytest.mark.parametrize("endian", ENDIAN) From b777d2e1c08580302003e83bac15fbe95d02b238 Mon Sep 17 00:00:00 2001 From: Davis Vann Bennett Date: Mon, 13 Jul 2026 15:37:01 +0200 Subject: [PATCH 3/4] 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 --- src/zarr/codecs/bytes.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/zarr/codecs/bytes.py b/src/zarr/codecs/bytes.py index 55d237d213..14fed9a860 100644 --- a/src/zarr/codecs/bytes.py +++ b/src/zarr/codecs/bytes.py @@ -103,21 +103,21 @@ def _decode_sync( 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 reinterpreted in - # the stored byte order, then converted to the declared dtype if needed. + # 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): - stored_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. - stored_dtype = dtype.newbyteorder(endian_str) + view_dtype = dtype.newbyteorder(endian_str) else: - stored_dtype = 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=stored_dtype) # type: ignore[attr-defined] + as_array_like.view(dtype=view_dtype) # type: ignore[attr-defined] ) - if stored_dtype != dtype: + if view_dtype != dtype: chunk_array = chunk_array.astype(dtype) # ensure correct chunk shape From e110bf6cf8a29923a9ea95f29698fd657f9cf4fd Mon Sep 17 00:00:00 2001 From: Davis Vann Bennett Date: Mon, 13 Jul 2026 15:37:45 +0200 Subject: [PATCH 4/4] docs: note that the decode-side byte-order conversion copies the chunk Assisted-by: ClaudeCode:claude-fable-5 --- src/zarr/codecs/bytes.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/zarr/codecs/bytes.py b/src/zarr/codecs/bytes.py index 14fed9a860..fae762fd08 100644 --- a/src/zarr/codecs/bytes.py +++ b/src/zarr/codecs/bytes.py @@ -118,6 +118,9 @@ def _decode_sync( 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