fix dtype sizes, adjust fill value parsing in from_dict, fix tests

Author: d-v-b

src/zarr/core/metadata/dtype.py

@@ -34,12 +34,15 @@ def endianness_to_numpy_str(endianness: Endianness | None) -> Literal[">", "<",
 def check_json_bool(data: JSON) -> TypeGuard[bool]:
     return bool(isinstance(data, bool))
 
+
 def check_json_str(data: JSON) -> TypeGuard[str]:
     return bool(isinstance(data, str))
 
+
 def check_json_int(data: JSON) -> TypeGuard[int]:
     return bool(isinstance(data, int))
 
+
 def check_json_float(data: JSON) -> TypeGuard[float]:
     if data == "NaN" or data == "Infinity" or data == "-Infinity":
         return True
@@ -254,7 +257,7 @@ def from_json_value(
 @dataclass(frozen=True, kw_only=True)
 class UInt8(DTypeBase):
     name = "uint8"
-    item_size = 2
+    item_size = 1
     kind = "numeric"
     numpy_character_code = "B"
     default = 0
@@ -488,13 +491,15 @@ def to_numpy(self, *, endianness: Endianness | None = None) -> np.dtypes.Float64
         return super().to_numpy(endianness=endianness)
 
     def to_json_value(self, data: np.generic, zarr_format: ZarrFormat) -> float:
-        return float(data)
+        return float_to_json(data, zarr_format)
 
     def from_json_value(
         self, data: JSON, *, zarr_format: ZarrFormat, endianness: Endianness | None = None
     ) -> np.float64:
         if check_json_float(data):
-            return float_from_json(data, dtype=self.to_numpy(endianness=endianness))
+            return float_from_json(
+                data, dtype=self.to_numpy(endianness=endianness), zarr_format=zarr_format
+            )
         raise TypeError(f"Invalid type: {data}. Expected a float.")
 
 
@@ -504,7 +509,7 @@ def from_json_value(
 @dataclass(frozen=True, kw_only=True)
 class Complex64(DTypeBase):
     name = "complex64"
-    item_size = 16
+    item_size = 8
     kind = "numeric"
     numpy_character_code = "F"
     default = 0.0 + 0.0j
@@ -533,7 +538,7 @@ def from_json_value(
 @dataclass(frozen=True, kw_only=True)
 class Complex128(DTypeBase):
     name = "complex128"
-    item_size = 32
+    item_size = 16
     kind = "numeric"
     numpy_character_code = "D"
     default = 0.0 + 0.0j

src/zarr/core/metadata/v3.py

@@ -263,28 +263,26 @@ def __init__(
         Because the class is a frozen dataclass, we set attributes using object.__setattr__
         """
         shape_parsed = parse_shapelike(shape)
-        data_type_parsed = data_type
         chunk_grid_parsed = ChunkGrid.from_dict(chunk_grid)
         chunk_key_encoding_parsed = ChunkKeyEncoding.from_dict(chunk_key_encoding)
         dimension_names_parsed = parse_dimension_names(dimension_names)
-        # we pass a string here rather than an enum to make mypy happy
-        fill_value_parsed = data_type_parsed.to_numpy().type(fill_value)
+        fill_value_parsed = data_type.to_numpy().type(fill_value)
         attributes_parsed = parse_attributes(attributes)
         codecs_parsed_partial = parse_codecs(codecs)
         storage_transformers_parsed = parse_storage_transformers(storage_transformers)
 
         array_spec = ArraySpec(
             shape=shape_parsed,
-            dtype=data_type_parsed.to_numpy(),
+            dtype=data_type.to_numpy(),
             fill_value=fill_value_parsed,
             config=ArrayConfig.from_dict({}),  # TODO: config is not needed here.
             prototype=default_buffer_prototype(),  # TODO: prototype is not needed here.
         )
         codecs_parsed = tuple(c.evolve_from_array_spec(array_spec) for c in codecs_parsed_partial)
-        validate_codecs(codecs_parsed_partial, data_type_parsed)
+        validate_codecs(codecs_parsed_partial, data_type)
 
         object.__setattr__(self, "shape", shape_parsed)
-        object.__setattr__(self, "data_type", data_type_parsed)
+        object.__setattr__(self, "data_type", data_type)
         object.__setattr__(self, "chunk_grid", chunk_grid_parsed)
         object.__setattr__(self, "chunk_key_encoding", chunk_key_encoding_parsed)
         object.__setattr__(self, "codecs", codecs_parsed)
@@ -405,11 +403,16 @@ def from_dict(cls, data: dict[str, JSON]) -> Self:
         else:
             data_type = get_data_type_from_dict(data_type_json)
 
+        # check that the fill value is consistent with the data type
+        fill_value_parsed = data_type.from_json_value(_data.pop("fill_value"), zarr_format=3)
+
         # dimension_names key is optional, normalize missing to `None`
         _data["dimension_names"] = _data.pop("dimension_names", None)
+
         # attributes key is optional, normalize missing to `None`
         _data["attributes"] = _data.pop("attributes", None)
-        return cls(**_data, data_type=data_type)  # type: ignore[arg-type]
+
+        return cls(**_data, fill_value=fill_value_parsed, data_type=data_type)  # type: ignore[arg-type]
 
     def to_dict(self) -> dict[str, JSON]:
         out_dict = super().to_dict()

tests/test_metadata/test_v3.py

@@ -12,7 +12,7 @@
 from zarr.core.chunk_key_encodings import DefaultChunkKeyEncoding, V2ChunkKeyEncoding
 from zarr.core.config import config
 from zarr.core.group import GroupMetadata, parse_node_type
-from zarr.core.metadata.dtype import complex_from_json
+from zarr.core.metadata.dtype import Flexible, complex_from_json
 from zarr.core.metadata.v3 import (
     ArrayV3Metadata,
     parse_dimension_names,
@@ -278,7 +278,7 @@ async def test_datetime_metadata(fill_value: int, precision: str) -> None:
 
 
 @pytest.mark.parametrize(
-    ("data_type", "fill_value"), [("uint8", -1), ("int32", 22.5), ("float32", "foo")]
+    ("data_type", "fill_value"), [("uint8", {}), ("int32", [0, 1]), ("float32", "foo")]
 )
 async def test_invalid_fill_value_raises(data_type: str, fill_value: float) -> None:
     metadata_dict = {
@@ -288,10 +288,11 @@ async def test_invalid_fill_value_raises(data_type: str, fill_value: float) -> N
         "chunk_grid": {"name": "regular", "configuration": {"chunk_shape": (1,)}},
         "data_type": data_type,
         "chunk_key_encoding": {"name": "default", "separator": "."},
-        "codecs": (),
+        "codecs": ({"name": "bytes"},),
         "fill_value": fill_value,  # this is not a valid fill value for uint8
     }
-    with pytest.raises(ValueError, match=r"fill value .* is not valid for dtype .*"):
+    # multiple things can go wrong here, so we don't match on the error message.
+    with pytest.raises(TypeError):
         ArrayV3Metadata.from_dict(metadata_dict)
 
 
@@ -323,13 +324,12 @@ async def test_special_float_fill_values(fill_value: str) -> None:
 
 @pytest.mark.parametrize("dtype_str", dtypes)
 def test_dtypes(dtype_str: str) -> None:
-    dt = DataType(dtype_str)
+    dt = get_data_type_from_numpy(dtype_str)
     np_dtype = dt.to_numpy()
-    if dtype_str not in vlen_dtypes:
-        # we can round trip "normal" dtypes
-        assert dt == DataType.from_numpy(np_dtype)
-        assert dt.byte_count == np_dtype.itemsize
-        assert dt.has_endianness == (dt.byte_count > 1)
+
+    if not isinstance(dt, Flexible):
+        assert dt.item_size == np_dtype.itemsize
     else:
-        # return type for vlen types may vary depending on numpy version
-        assert dt.byte_count is None
+        assert dt.length == np_dtype.itemsize
+
+    assert dt.numpy_character_code == np_dtype.char