From ff62fb2225febd2e3a081f0f67a4b413a1d77c18 Mon Sep 17 00:00:00 2001 From: Sebastian Hoffmann Date: Thu, 23 Jul 2026 13:45:00 +0200 Subject: [PATCH 1/5] fix(ArraySpec): proper and robust equality semantics for ArraySpec by checking fill_value for byte-identicality, fixes #3054. --- src/zarr/core/array_spec.py | 22 ++++- tests/test_array_spec.py | 168 ++++++++++++++++++++++++++++++++++++ 2 files changed, 189 insertions(+), 1 deletion(-) create mode 100644 tests/test_array_spec.py diff --git a/src/zarr/core/array_spec.py b/src/zarr/core/array_spec.py index 89163f7d83..1f4ffd6f09 100644 --- a/src/zarr/core/array_spec.py +++ b/src/zarr/core/array_spec.py @@ -3,6 +3,8 @@ from dataclasses import dataclass, fields from typing import TYPE_CHECKING, Any, Literal, Self, TypedDict, cast +import numpy as np + from zarr.core.common import ( MemoryOrder, parse_bool, @@ -132,7 +134,7 @@ def parse_array_config(data: ArrayConfigLike | None) -> ArrayConfig: return ArrayConfig.from_dict(data) -@dataclass(frozen=True) +@dataclass(frozen=True, eq=False) class ArraySpec: shape: tuple[int, ...] dtype: ZDType[TBaseDType, TBaseScalar] @@ -157,6 +159,24 @@ def __init__( object.__setattr__(self, "config", config) object.__setattr__(self, "prototype", prototype) + def _key(self) -> tuple[object, ...]: + """Returns the tuple used for equality/hash identity.""" + fill_value = self.fill_value + if isinstance(fill_value, np.generic): + # fill_values should be byte-identical, otherwise they correspond to different values in memory / on disk. + # Importantly, this ensures np.nan == np.nan, NaT == NaT, and -0.0 != 0.0. + # It also fixes np.void fill_values being unhashable (#3054). + fill_value = fill_value.tobytes() + return (self.shape, self.dtype, fill_value, self.config, self.prototype) + + def __eq__(self, other: object) -> bool: + if not isinstance(other, ArraySpec): + return NotImplemented + return self._key() == other._key() + + def __hash__(self) -> int: + return hash(self._key()) + @property def ndim(self) -> int: return len(self.shape) diff --git a/tests/test_array_spec.py b/tests/test_array_spec.py new file mode 100644 index 0000000000..d04636e8f2 --- /dev/null +++ b/tests/test_array_spec.py @@ -0,0 +1,168 @@ +from __future__ import annotations + +from typing import TYPE_CHECKING, Any + +import numpy as np +import pytest + +from zarr.core.array_spec import ArrayConfig, ArraySpec +from zarr.core.buffer import BufferPrototype, default_buffer_prototype +from zarr.core.buffer.cpu import NDBuffer +from zarr.core.dtype import get_data_type_from_native_dtype + +if TYPE_CHECKING: + from collections.abc import Callable + + from zarr.core.common import MemoryOrder + + +def _make_spec( + *, + shape: tuple[int, ...] = (4, 4), + native_dtype: Any = "int16", + fill_value: Any = 0, + order: MemoryOrder = "C", + write_empty_chunks: bool = False, + prototype: BufferPrototype | None = None, +) -> ArraySpec: + """Creates an ArraySpec with common defaults""" + zdtype = get_data_type_from_native_dtype(np.dtype(native_dtype)) + fill_value = zdtype.cast_scalar(fill_value) # mirrors ArrayV3Metadata's fill_value + return ArraySpec( + shape=shape, + dtype=zdtype, + fill_value=fill_value, + config=ArrayConfig(order=order, write_empty_chunks=write_empty_chunks), + prototype=prototype if prototype is not None else default_buffer_prototype(), + ) + + +class _AltNDBuffer(NDBuffer): + """A distinct NDBuffer subclass""" + + +_ALT_PROTOTYPE = BufferPrototype( + buffer=default_buffer_prototype().buffer, + nd_buffer=_AltNDBuffer, +) # a distinct BufferPrototype with a different nd_buffer subclass + + +# Difficult / important cases: +# issue #3054: np.void is unhashable when writeable +# nan/NaT aren't self-equal yet must compare equal for a ArraySpec +SPECS = [ + pytest.param({"native_dtype": "int16", "fill_value": 7}, id="int16"), + pytest.param({"native_dtype": "float64", "fill_value": 1.5}, id="float64"), + pytest.param({"native_dtype": "float64", "fill_value": float("nan")}, id="float64-nan"), + pytest.param({"native_dtype": "float64", "fill_value": -0.0}, id="float64-negzero"), + pytest.param({"native_dtype": "complex128", "fill_value": 1 + 2j}, id="complex128"), + pytest.param( + {"native_dtype": "complex128", "fill_value": complex(-0.0, -0.0)}, + id="complex128-negzero", + ), + pytest.param({"native_dtype": "bool", "fill_value": True}, id="bool"), + pytest.param( + {"native_dtype": "datetime64[s]", "fill_value": np.datetime64("2020-01-01")}, + id="datetime64", + ), + pytest.param( + {"native_dtype": "datetime64[s]", "fill_value": np.datetime64("NaT", "s")}, + id="datetime64-NaT", + ), + pytest.param( + {"native_dtype": [("a", "f8"), ("b", "i8")], "fill_value": (1.0, 2)}, + id="structured-void", + ), + pytest.param({"native_dtype": "U5", "fill_value": "hello"}, id="fixed-string"), + pytest.param({"shape": ()}, id="scalar-shape"), + pytest.param({"shape": (0,)}, id="zero-size"), + pytest.param({"order": "F"}, id="order-F"), +] + + +# Mutations: each mutate kwargs to an uneqal version +def _grow_shape(kw: dict[str, Any]) -> dict[str, Any]: + return {"shape": (*kw.get("shape", (4, 4)), 1)} + + +def _flip_order(kw: dict[str, Any]) -> dict[str, Any]: + return {"order": "F" if kw.get("order", "C") == "C" else "C"} + + +def _swap_prototype(_kw: dict[str, Any]) -> dict[str, Any]: + return {"prototype": _ALT_PROTOTYPE} + + +MUTATIONS = [ + pytest.param(_grow_shape, id="shape"), + pytest.param(_flip_order, id="order"), + pytest.param(_swap_prototype, id="prototype"), +] + + +class TestArraySpecHashEq: + @pytest.mark.parametrize("kwargs", SPECS) + def test_hashable(self, kwargs: dict[str, Any]) -> None: + """Every ArraySpec is hashable, including structured (np.void) fill values.""" + assert isinstance(hash(_make_spec(**kwargs)), int) + + @pytest.mark.parametrize("kwargs", SPECS) + def test_equal_specs_hash_equal(self, kwargs: dict[str, Any]) -> None: + """Independently built specs with identical fields are equal and hash equal.""" + a = _make_spec(**kwargs) + b = _make_spec(**kwargs) + assert a == b + assert hash(a) == hash(b) + + @pytest.mark.parametrize("kwargs", SPECS) + @pytest.mark.parametrize("mutate", MUTATIONS) + def test_distinct_specs_unequal( + self, + mutate: Callable[[dict[str, Any]], dict[str, Any]], + kwargs: dict[str, Any], + ) -> None: + """Changing one dtype-independent field makes a spec unequal to its base.""" + base = _make_spec(**kwargs) + variant = _make_spec(**{**kwargs, **mutate(kwargs)}) + assert base != variant + assert hash(base) != hash(variant) + + @pytest.mark.parametrize( + ("base", "variant"), + [ + pytest.param({"fill_value": 0}, {"fill_value": 1}, id="fill_value"), + pytest.param({"native_dtype": "int16"}, {"native_dtype": "int32"}, id="dtype"), + pytest.param( + {"native_dtype": "float32", "fill_value": 1.0}, + {"native_dtype": "float64", "fill_value": 1.0}, + id="dtype-float-promote", + ), + ], + ) + def test_dtype_and_fill_value_matter( + self, base: dict[str, Any], variant: dict[str, Any] + ) -> None: + """dtype and fill_value participate in equality; they can't join the cross + product because fill_value is coupled to dtype.""" + assert _make_spec(**base) != _make_spec(**variant) + + @pytest.mark.parametrize( + ("native_dtype", "neg_fill", "pos_fill"), + [ + pytest.param("float16", -0.0, 0.0, id="float16"), + pytest.param("float32", -0.0, 0.0, id="float32"), + pytest.param("float64", -0.0, 0.0, id="float64"), + pytest.param("complex128", complex(-0.0, -0.0), 0j, id="complex128-both"), + pytest.param("complex128", complex(0.0, -0.0), 0j, id="complex128-imag"), + pytest.param("complex128", complex(-0.0, 0.0), 0j, id="complex128-real"), + pytest.param([("a", "f8")], (-0.0,), (0.0,), id="structured"), + ], + ) + def test_signed_zero_fills_are_distinct( + self, native_dtype: Any, neg_fill: Any, pos_fill: Any + ) -> None: + """A -0.0 fill writes different bytes than +0.0, so the specs are not equal.""" + neg = _make_spec(native_dtype=native_dtype, fill_value=neg_fill) + pos = _make_spec(native_dtype=native_dtype, fill_value=pos_fill) + assert neg != pos + assert hash(neg) != hash(pos) From 55f096eb6c1bde11f94daa1257b849c5f0635f53 Mon Sep 17 00:00:00 2001 From: Sebastian Hoffmann Date: Thu, 23 Jul 2026 14:34:22 +0200 Subject: [PATCH 2/5] fix: added extra case for unequal types --- tests/test_array_spec.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/tests/test_array_spec.py b/tests/test_array_spec.py index d04636e8f2..d46591fb86 100644 --- a/tests/test_array_spec.py +++ b/tests/test_array_spec.py @@ -166,3 +166,17 @@ def test_signed_zero_fills_are_distinct( pos = _make_spec(native_dtype=native_dtype, fill_value=pos_fill) assert neg != pos assert hash(neg) != hash(pos) + + @pytest.mark.parametrize( + ("obj"), + [ + pytest.param(None, id="None"), + pytest.param(42, id="int"), + pytest.param("hello", id="str"), + pytest.param([1, 2, 3], id="list"), + pytest.param({"a": 1}, id="dict"), + ], + ) + def test_unequal_with_invalid_type(self, obj: Any) -> None: + assert (_make_spec() == obj) is False + assert _make_spec() != obj From 6e58a51ef9c038b9ac591777ae175a997abc9699 Mon Sep 17 00:00:00 2001 From: Davis Bennett Date: Tue, 28 Jul 2026 17:26:29 +0200 Subject: [PATCH 3/5] Update tests/test_array_spec.py --- tests/test_array_spec.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_array_spec.py b/tests/test_array_spec.py index d46591fb86..7fc640319c 100644 --- a/tests/test_array_spec.py +++ b/tests/test_array_spec.py @@ -80,7 +80,7 @@ class _AltNDBuffer(NDBuffer): ] -# Mutations: each mutate kwargs to an uneqal version +# Mutations: each mutate kwargs to an unequal version def _grow_shape(kw: dict[str, Any]) -> dict[str, Any]: return {"shape": (*kw.get("shape", (4, 4)), 1)} From c4c8df7f81d19ef04c7ffdfb9e3ff7af81c5c274 Mon Sep 17 00:00:00 2001 From: Sebastian Hoffmann Date: Wed, 29 Jul 2026 12:51:47 +0200 Subject: [PATCH 4/5] addressed reviewers comments --- src/zarr/codecs/sharding.py | 8 +- tests/test_array_spec.py | 157 ++++++++++++++++++------------------ 2 files changed, 79 insertions(+), 86 deletions(-) diff --git a/src/zarr/codecs/sharding.py b/src/zarr/codecs/sharding.py index 2d4d63d400..f20979066d 100644 --- a/src/zarr/codecs/sharding.py +++ b/src/zarr/codecs/sharding.py @@ -411,11 +411,7 @@ def __init__( object.__setattr__(self, "subchunk_write_order", subchunk_write_order) # Use instance-local lru_cache to avoid memory leaks - - # numpy void scalars are not hashable, which means an array spec with a fill value that is - # a numpy void scalar will break the lru_cache. This is commented for now but should be - # fixed. See https://github.com/zarr-developers/zarr-python/issues/3054 - # object.__setattr__(self, "_get_chunk_spec", lru_cache()(self._get_chunk_spec)) + object.__setattr__(self, "_get_chunk_spec", lru_cache()(self._get_chunk_spec)) object.__setattr__(self, "_get_index_chunk_spec", lru_cache()(self._get_index_chunk_spec)) object.__setattr__(self, "_get_chunks_per_shard", lru_cache()(self._get_chunks_per_shard)) object.__setattr__(self, "_shard_index_size", lru_cache()(self._shard_index_size)) @@ -441,7 +437,7 @@ def __setstate__(self, state: dict[str, Any]) -> None: object.__setattr__(self, "subchunk_write_order", state["subchunk_write_order"]) # Use instance-local lru_cache to avoid memory leaks - # object.__setattr__(self, "_get_chunk_spec", lru_cache()(self._get_chunk_spec)) + object.__setattr__(self, "_get_chunk_spec", lru_cache()(self._get_chunk_spec)) object.__setattr__(self, "_get_index_chunk_spec", lru_cache()(self._get_index_chunk_spec)) object.__setattr__(self, "_get_chunks_per_shard", lru_cache()(self._get_chunks_per_shard)) object.__setattr__(self, "_shard_index_size", lru_cache()(self._shard_index_size)) diff --git a/tests/test_array_spec.py b/tests/test_array_spec.py index 7fc640319c..4fbc0b1205 100644 --- a/tests/test_array_spec.py +++ b/tests/test_array_spec.py @@ -100,83 +100,80 @@ def _swap_prototype(_kw: dict[str, Any]) -> dict[str, Any]: ] -class TestArraySpecHashEq: - @pytest.mark.parametrize("kwargs", SPECS) - def test_hashable(self, kwargs: dict[str, Any]) -> None: - """Every ArraySpec is hashable, including structured (np.void) fill values.""" - assert isinstance(hash(_make_spec(**kwargs)), int) - - @pytest.mark.parametrize("kwargs", SPECS) - def test_equal_specs_hash_equal(self, kwargs: dict[str, Any]) -> None: - """Independently built specs with identical fields are equal and hash equal.""" - a = _make_spec(**kwargs) - b = _make_spec(**kwargs) - assert a == b - assert hash(a) == hash(b) - - @pytest.mark.parametrize("kwargs", SPECS) - @pytest.mark.parametrize("mutate", MUTATIONS) - def test_distinct_specs_unequal( - self, - mutate: Callable[[dict[str, Any]], dict[str, Any]], - kwargs: dict[str, Any], - ) -> None: - """Changing one dtype-independent field makes a spec unequal to its base.""" - base = _make_spec(**kwargs) - variant = _make_spec(**{**kwargs, **mutate(kwargs)}) - assert base != variant - assert hash(base) != hash(variant) - - @pytest.mark.parametrize( - ("base", "variant"), - [ - pytest.param({"fill_value": 0}, {"fill_value": 1}, id="fill_value"), - pytest.param({"native_dtype": "int16"}, {"native_dtype": "int32"}, id="dtype"), - pytest.param( - {"native_dtype": "float32", "fill_value": 1.0}, - {"native_dtype": "float64", "fill_value": 1.0}, - id="dtype-float-promote", - ), - ], - ) - def test_dtype_and_fill_value_matter( - self, base: dict[str, Any], variant: dict[str, Any] - ) -> None: - """dtype and fill_value participate in equality; they can't join the cross - product because fill_value is coupled to dtype.""" - assert _make_spec(**base) != _make_spec(**variant) - - @pytest.mark.parametrize( - ("native_dtype", "neg_fill", "pos_fill"), - [ - pytest.param("float16", -0.0, 0.0, id="float16"), - pytest.param("float32", -0.0, 0.0, id="float32"), - pytest.param("float64", -0.0, 0.0, id="float64"), - pytest.param("complex128", complex(-0.0, -0.0), 0j, id="complex128-both"), - pytest.param("complex128", complex(0.0, -0.0), 0j, id="complex128-imag"), - pytest.param("complex128", complex(-0.0, 0.0), 0j, id="complex128-real"), - pytest.param([("a", "f8")], (-0.0,), (0.0,), id="structured"), - ], - ) - def test_signed_zero_fills_are_distinct( - self, native_dtype: Any, neg_fill: Any, pos_fill: Any - ) -> None: - """A -0.0 fill writes different bytes than +0.0, so the specs are not equal.""" - neg = _make_spec(native_dtype=native_dtype, fill_value=neg_fill) - pos = _make_spec(native_dtype=native_dtype, fill_value=pos_fill) - assert neg != pos - assert hash(neg) != hash(pos) - - @pytest.mark.parametrize( - ("obj"), - [ - pytest.param(None, id="None"), - pytest.param(42, id="int"), - pytest.param("hello", id="str"), - pytest.param([1, 2, 3], id="list"), - pytest.param({"a": 1}, id="dict"), - ], - ) - def test_unequal_with_invalid_type(self, obj: Any) -> None: - assert (_make_spec() == obj) is False - assert _make_spec() != obj +@pytest.mark.parametrize("kwargs", SPECS) +def test_hashable(kwargs: dict[str, Any]) -> None: + """Every ArraySpec is hashable, including structured (np.void) fill values.""" + assert isinstance(hash(_make_spec(**kwargs)), int) + + +@pytest.mark.parametrize("kwargs", SPECS) +def test_equal_specs_hash_equal(kwargs: dict[str, Any]) -> None: + """Independently built specs with identical fields are equal and hash equal.""" + a = _make_spec(**kwargs) + b = _make_spec(**kwargs) + assert a == b + assert hash(a) == hash(b) + + +@pytest.mark.parametrize("kwargs", SPECS) +@pytest.mark.parametrize("mutate", MUTATIONS) +def test_distinct_specs_unequal( + mutate: Callable[[dict[str, Any]], dict[str, Any]], + kwargs: dict[str, Any], +) -> None: + """Changing one dtype-independent field makes a spec unequal to its base.""" + base = _make_spec(**kwargs) + variant = _make_spec(**{**kwargs, **mutate(kwargs)}) + assert base != variant + + +@pytest.mark.parametrize( + ("base", "variant"), + [ + pytest.param({"fill_value": 0}, {"fill_value": 1}, id="fill_value"), + pytest.param({"native_dtype": "int16"}, {"native_dtype": "int32"}, id="dtype"), + pytest.param( + {"native_dtype": "float32", "fill_value": 1.0}, + {"native_dtype": "float64", "fill_value": 1.0}, + id="dtype-float-promote", + ), + ], +) +def test_dtype_and_fill_value_matter(base: dict[str, Any], variant: dict[str, Any]) -> None: + """dtype and fill_value participate in equality; they can't join the cross + product because fill_value is coupled to dtype.""" + assert _make_spec(**base) != _make_spec(**variant) + + +@pytest.mark.parametrize( + ("native_dtype", "neg_fill", "pos_fill"), + [ + pytest.param("float16", -0.0, 0.0, id="float16"), + pytest.param("float32", -0.0, 0.0, id="float32"), + pytest.param("float64", -0.0, 0.0, id="float64"), + pytest.param("complex128", complex(-0.0, -0.0), 0j, id="complex128-both"), + pytest.param("complex128", complex(0.0, -0.0), 0j, id="complex128-imag"), + pytest.param("complex128", complex(-0.0, 0.0), 0j, id="complex128-real"), + pytest.param([("a", "f8")], (-0.0,), (0.0,), id="structured"), + ], +) +def test_signed_zero_fills_are_distinct(native_dtype: Any, neg_fill: Any, pos_fill: Any) -> None: + """A -0.0 fill writes different bytes than +0.0, so the specs are not equal.""" + neg = _make_spec(native_dtype=native_dtype, fill_value=neg_fill) + pos = _make_spec(native_dtype=native_dtype, fill_value=pos_fill) + assert neg != pos + + +@pytest.mark.parametrize( + ("obj"), + [ + pytest.param(None, id="None"), + pytest.param(42, id="int"), + pytest.param("hello", id="str"), + pytest.param([1, 2, 3], id="list"), + pytest.param({"a": 1}, id="dict"), + ], +) +def test_unequal_with_invalid_type(obj: Any) -> None: + assert (_make_spec() == obj) is False + assert _make_spec() != obj From fb552ebcff721bdccc91f2384f93ec96652a5c3d Mon Sep 17 00:00:00 2001 From: Davis Vann Bennett Date: Wed, 29 Jul 2026 13:26:32 +0200 Subject: [PATCH 5/5] test: add end-to-end regression test for structured-dtype fills in sharded arrays, and changelog entry The new test exercises the sharding codec's chunk-spec caches with an unhashable np.void fill value (#3054), which the ArraySpec test suite only covers at the unit level. Assisted-by: ClaudeCode:claude-fable-5 --- changes/4183.bugfix.md | 3 +++ tests/test_codecs/test_sharding.py | 23 +++++++++++++++++++++++ 2 files changed, 26 insertions(+) create mode 100644 changes/4183.bugfix.md diff --git a/changes/4183.bugfix.md b/changes/4183.bugfix.md new file mode 100644 index 0000000000..809708f596 --- /dev/null +++ b/changes/4183.bugfix.md @@ -0,0 +1,3 @@ +Fixed `TypeError: unhashable type: 'writeable void-scalar'` when writing to sharded arrays whose fill value is a `np.void` scalar, e.g. arrays with a structured dtype. + +`ArraySpec` equality and hashing now compare the fill value by its byte representation rather than numeric equality. As a result, two specs with a `NaN` (or `NaT`) fill value now compare equal, while fill values of `-0.0` and `0.0` now compare unequal. This also restores the sharding codec's per-chunk spec cache, which had been disabled because of this bug. diff --git a/tests/test_codecs/test_sharding.py b/tests/test_codecs/test_sharding.py index 9e6bebd8df..de576dbef5 100644 --- a/tests/test_codecs/test_sharding.py +++ b/tests/test_codecs/test_sharding.py @@ -734,6 +734,29 @@ async def test_delete_empty_shards(store: Store) -> None: assert len(chunk_bytes) == 16 * 2 + 8 * 8 * 2 + 4 +def test_structured_dtype_fill_value() -> None: + """Sharded arrays with a structured dtype are writable and readable even though + the fill value is an (unhashable) ``np.void`` scalar: the sharding codec's + chunk-spec caches key on ``ArraySpec``, whose hash must handle void fills + (see https://github.com/zarr-developers/zarr-python/issues/3054).""" + dtype = np.dtype([("a", "i4"), ("b", "f4")]) + arr = zarr.create_array( + MemoryStore(), + shape=(8,), + chunks=(2,), + shards=(4,), + dtype=dtype, + fill_value=(1, 2.0), + ) + data = np.array([(i, i / 2) for i in range(8)], dtype=dtype) + arr[:4] = data[:4] + + expected = np.zeros(8, dtype=dtype) + expected[:4] = data[:4] + expected[4:] = (1, 2.0) # untouched shard reads back as the fill value + assert np.array_equal(arr[:], expected) + + def test_pickle() -> None: """ShardingCodec round-trips through pickle, including the non-serialized ``subchunk_write_order`` (which ``to_dict`` omits and which must not silently