fix: preserve non-canonical NaN payloads and distinguish -0.0 in v3 metadata

changes/4486.bugfix.md

@@ -0,0 +1,11 @@
+Preserve non-canonical NaN payloads in v3 fill values. Only the exact
+canonical NaN bit pattern, sign bit included, serializes as `NaN`; every
+other NaN, including the negative-sign form of the canonical payload,
+writes back as its hex bits, and hex strings decode into same-width
+NumPy scalars so float16 payloads survive. `ArrayV3Metadata` equality
+now compares the serialized JSON text without sorting keys, so attribute
+dictionaries with mixed key types no longer raise `TypeError`; as a
+consequence of comparing JSON text, attributes that differ only in JSON
+spelling, such as `1` and `1.0`, no longer compare equal. This also
+stops equality from conflating `-0.0` with `0.0`. Partial fix for issue
+#4453.

src/zarr/core/dtype/npy/common.py

@@ -1,7 +1,6 @@
 from __future__ import annotations
 
 import base64
-import struct
 import sys
 from collections.abc import Sequence
 from typing import (
@@ -15,6 +14,7 @@
     SupportsIndex,
     SupportsInt,
     TypeGuard,
+    cast,
 )
 
 import numpy as np
@@ -162,7 +162,7 @@ def float_from_json_v2(data: JSONFloatV2) -> float:
             return float(data)
 
 
-def float_from_json_v3(data: JSONFloatV3) -> float:
+def float_from_json_v3(data: JSONFloatV3) -> float | np.floating[Any]:
     """
     Convert a JSON float to a float (v3).
 
@@ -182,6 +182,9 @@ def float_from_json_v3(data: JSONFloatV3) -> float:
        "...for float32, "NaN" is equivalent to "0x7fc00000".
        This representation is the only way to specify a NaN value other than the specific NaN value
        denoted by "NaN"."
+
+    Hex strings decode into a NumPy scalar of the matching width rather than a
+    Python float, so a non-canonical NaN payload survives the parse.
     """
 
     if isinstance(data, str):
@@ -194,18 +197,21 @@ def float_from_json_v3(data: JSONFloatV3) -> float:
             )
             raise ValueError(msg)
         if len(data[2:]) == 4:
-            dtype_code = ">e"
+            dtype_code = ">f2"
         elif len(data[2:]) == 8:
-            dtype_code = ">f"
+            dtype_code = ">f4"
         elif len(data[2:]) == 16:
-            dtype_code = ">d"
+            dtype_code = ">f8"
         else:
             msg = (
                 f"Invalid hexadecimal float value: {data!r}. "
                 "Expected the '0x' prefix to be followed by 4, 8, or 16 numeral characters"
             )
             raise ValueError(msg)
-        return float(struct.unpack(dtype_code, bytes.fromhex(data[2:]))[0])
+        return cast(
+            np.floating[Any],
+            np.frombuffer(bytes.fromhex(data[2:]), dtype=dtype_code)[0],
+        )
     return float_from_json_v2(data)
 
 
@@ -287,9 +293,23 @@ def float_to_json_v3(data: float | np.floating[Any]) -> JSONFloatV3:
     -------
     JSONFloat
         The JSON representation of the float.
-    """
-    # v3 can in principle handle distinct NaN values, but numpy does not represent these explicitly
-    # so we just reuse the v2 routine here
+
+    Notes
+    -----
+    ``"NaN"`` denotes exactly the canonical NaN bit pattern, sign bit
+    included. Any other NaN, including the negative-sign form of the
+    canonical payload, is written as its hexadecimal bit pattern per the
+    spec's hex-bits encoding (``"0x7fc00001"`` for a float32 NaN with
+    payload 1), which is the only way to name a non-canonical NaN.
+    """
+    if np.isnan(data) and isinstance(data, np.floating):
+        uint_dtype = {2: np.uint16, 4: np.uint32, 8: np.uint64}.get(data.dtype.itemsize)
+        if uint_dtype is not None:
+            bits = int(np.asarray(data).view(uint_dtype))
+            canonical = int(np.asarray(np.nan, dtype=data.dtype).view(uint_dtype))
+            if bits == canonical:
+                return float_to_json_v2(data)
+            return f"0x{bits:0{data.dtype.itemsize * 2}x}"
     return float_to_json_v2(data)
 
 

src/zarr/core/metadata/v3.py

@@ -759,14 +759,17 @@ def __eq__(self, other: object) -> bool:
         # The default dataclass __eq__ compares fields directly, which is wrong for a NaN
         # fill_value: NaN != NaN under IEEE 754. Comparing the JSON-serialized form instead
         # treats matching NaN (and inf) fill values as equal. See issue #2929.
+        # The comparison uses the serialized JSON *text*: dict equality conflates
+        # -0.0 and 0.0, and a non-canonical NaN payload with "NaN", though the
+        # documents write back differently. See issue #4453.
         if not isinstance(other, ArrayV3Metadata):
             return NotImplemented
-        return self.to_dict() == other.to_dict()
+        return json.dumps(self.to_dict()) == json.dumps(other.to_dict())
 
     def __hash__(self) -> int:
         # Hash the JSON-serialized form to stay consistent with __eq__: equal metadata
         # must hash equally, which a field-based hash violates for a NaN fill_value.
-        return hash(json.dumps(self.to_dict(), sort_keys=True))
+        return hash(json.dumps(self.to_dict()))
 
     def update_shape(self, shape: tuple[int, ...]) -> Self:
         chunk_grid = self.chunk_grid

tests/test_array.py

@@ -1037,17 +1037,26 @@ def test_write_empty_chunks_negative_zero(
         (np.nan * 1j, ["NaN", "NaN"]),
         (np.nan, ["NaN", 0.0]),
         (np.inf, ["Infinity", 0.0]),
-        (np.inf * 1j, ["NaN", "Infinity"]),
+        (np.inf * 1j, None),
         (-np.inf, ["-Infinity", 0.0]),
         (math.inf, ["Infinity", 0.0]),
     ],
 )
-async def test_special_complex_fill_values_roundtrip(fill_value: Any, expected: list[Any]) -> None:
+async def test_special_complex_fill_values_roundtrip(
+    fill_value: Any, expected: list[Any] | None
+) -> None:
     store = MemoryStore()
     zarr.create_array(store=store, shape=(1,), dtype=np.complex64, fill_value=fill_value)
     content = await store.get("zarr.json", prototype=default_buffer_prototype())
     assert content is not None
     actual = json.loads(content.to_bytes())
+    if expected is None:
+        # inf * 1j computes 0 * inf for the real part, which produces a NaN
+        # whose sign bit is platform-dependent. Only the exact canonical bits
+        # serialize as "NaN"; any other NaN writes as hex.
+        real_bits = int(np.array([np.complex64(fill_value)]).view(np.uint32)[0])
+        canonical = int(np.asarray(np.nan, dtype=np.float32).view(np.uint32))
+        expected = ["NaN" if real_bits == canonical else f"0x{real_bits:08x}", "Infinity"]
     assert actual["fill_value"] == expected
 
 

tests/test_dtype/test_npy/test_float.py

@@ -1,9 +1,12 @@
 from __future__ import annotations
 
+from typing import Any
+
 import numpy as np
+import pytest
 
 from tests.test_dtype.test_wrapper import BaseTestZDType
-from zarr.core.dtype.npy.float import Float16, Float32, Float64
+from zarr.core.dtype.npy.float import BaseFloat, Float16, Float32, Float64
 
 
 class _BaseTestFloat(BaseTestZDType):
@@ -223,3 +226,37 @@ def test_string_float_from_json_scalar() -> None:
     # Test that it works for v2 format too
     result = dtype_instance.from_json_scalar("1.5", zarr_format=2)
     assert result == np.float32(1.5)
+
+
+@pytest.mark.parametrize(
+    ("dtype", "hex_value"),
+    [
+        (Float16(), "0x7fc1"),
+        (Float32(), "0x7fc00001"),
+        (Float32(), "0xffc00000"),
+        (Float32(), "0xffc00001"),
+        (Float64(), "0x7ff8000000000001"),
+        (Float64(), "0xfff8000000000000"),
+    ],
+)
+def test_noncanonical_nan_serializes_as_hex(dtype: BaseFloat[Any, Any], hex_value: str) -> None:
+    """Only the exact canonical NaN bit pattern writes back as ``"NaN"``;
+    every other NaN, including the negative-sign form of the canonical
+    payload, writes back as its hex bits. Regression test for
+    https://github.com/zarr-developers/zarr-python/issues/4453 item 3."""
+    scalar = dtype.from_json_scalar(hex_value, zarr_format=3)
+    assert dtype.to_json_scalar(scalar, zarr_format=3) == hex_value
+    canonical = dtype.from_json_scalar("NaN", zarr_format=3)
+    assert dtype.to_json_scalar(canonical, zarr_format=3) == "NaN"
+
+
+def test_noncanonical_nan_serializes_as_hex_complex() -> None:
+    """The complex path applies the same canonical-NaN check to each part."""
+    from zarr.core.dtype.npy.complex import Complex64
+
+    dtype = Complex64()
+    real_nan = np.array([0xFFC00000], dtype=np.uint32).view(np.float32)[0]
+    scalar = np.complex64(complex(real_nan, 0))
+    assert dtype.to_json_scalar(scalar, zarr_format=3) == ("0xffc00000", 0.0)
+    canonical = np.complex64(complex(np.nan, np.nan))
+    assert dtype.to_json_scalar(canonical, zarr_format=3) == ("NaN", "NaN")

tests/test_metadata/test_v3.py

@@ -573,3 +573,31 @@ def test_group_metadata_to_dict_consolidated(attributes: dict[str, Any] | None)
             },
         },
     }
+
+
+def test_eq_distinguishes_signed_zero_fill_value() -> None:
+    """Documents with fill_value -0.0 and 0.0 write back differently and must
+    not compare equal. Regression test for issue #4453 item 3."""
+    neg = ArrayV3Metadata.from_dict(
+        minimal_metadata_dict_v3(data_type="float64", fill_value=-0.0)  # type: ignore[arg-type]
+    )
+    pos = ArrayV3Metadata.from_dict(
+        minimal_metadata_dict_v3(data_type="float64", fill_value=0.0)  # type: ignore[arg-type]
+    )
+    assert neg != pos
+    assert hash(neg) != hash(pos)
+
+
+def test_nan_payload_fill_value_round_trips() -> None:
+    """A non-canonical NaN payload survives a document round trip as its hex
+    bits, and does not compare equal to the canonical NaN document."""
+    payload = ArrayV3Metadata.from_dict(
+        minimal_metadata_dict_v3(data_type="float32", fill_value="0x7fc00001")  # type: ignore[arg-type]
+    )
+    canonical = ArrayV3Metadata.from_dict(
+        minimal_metadata_dict_v3(data_type="float32", fill_value="NaN")  # type: ignore[arg-type]
+    )
+    assert payload.to_dict()["fill_value"] == "0x7fc00001"
+    assert canonical.to_dict()["fill_value"] == "NaN"
+    assert payload != canonical
+    assert hash(payload) != hash(canonical)