From fe9cc85c389b4e67a60dbed117083e9183d44c38 Mon Sep 17 00:00:00 2001 From: Tom Augspurger Date: Tue, 1 Oct 2024 10:58:02 -0500 Subject: [PATCH 1/5] wip - fill value --- src/zarr/abc/metadata.py | 1 - src/zarr/core/metadata/v2.py | 30 +++++++++++++++++++++++++-- tests/v3/test_v2.py | 39 ++++++++++++++++++++++++++++++++++++ 3 files changed, 67 insertions(+), 3 deletions(-) diff --git a/src/zarr/abc/metadata.py b/src/zarr/abc/metadata.py index 239d151c0c..291ceb459c 100644 --- a/src/zarr/abc/metadata.py +++ b/src/zarr/abc/metadata.py @@ -22,7 +22,6 @@ def to_dict(self) -> dict[str, JSON]: are instances of `Metadata`. Sequences of `Metadata` are similarly recursed into, and the output of that recursion is collected in a list. """ - ... out_dict = {} for field in fields(self): key = field.name diff --git a/src/zarr/core/metadata/v2.py b/src/zarr/core/metadata/v2.py index df7f2abaea..16f5a09d3e 100644 --- a/src/zarr/core/metadata/v2.py +++ b/src/zarr/core/metadata/v2.py @@ -1,8 +1,9 @@ from __future__ import annotations +import base64 from collections.abc import Iterable from enum import Enum -from typing import TYPE_CHECKING +from typing import TYPE_CHECKING, cast if TYPE_CHECKING: from typing import Any, Literal, Self @@ -31,7 +32,7 @@ class ArrayV2Metadata(ArrayMetadata): shape: ChunkCoords chunk_grid: RegularChunkGrid data_type: np.dtype[Any] - fill_value: None | int | float = 0 + fill_value: None | int | float | str | bytes = 0 order: Literal["C", "F"] = "C" filters: tuple[numcodecs.abc.Codec, ...] | None = None dimension_separator: Literal[".", "/"] = "." @@ -140,10 +141,35 @@ def from_dict(cls, data: dict[str, Any]) -> ArrayV2Metadata: _data = data.copy() # check that the zarr_format attribute is correct _ = parse_zarr_format(_data.pop("zarr_format")) + dtype = parse_dtype(_data["dtype"]) + + if dtype.kind in "SV": + fill_value_encoded = _data.get("fill_value") + if fill_value_encoded is not None: + if dtype.kind == "S": + try: + fill_value = base64.standard_b64decode(fill_value_encoded) + _data["fill_value"] = fill_value + except Exception: + # be lenient, allow for other values that may have been used before base64 + # encoding and may work as fill values, e.g., the number 0 + pass + elif dtype.kind == "V": + fill_value = base64.standard_b64encode(fill_value_encoded) + _data["fill_value"] = fill_value + return cls(**_data) def to_dict(self) -> dict[str, JSON]: zarray_dict = super().to_dict() + + if self.dtype.kind in "SV": + # There's a relationship between self.dtype and self.fill_value + # that mypy isn't aware of. The fact that we have S or V dtype here + # means we should have a bytes-type fill_value. + fill_value = base64.standard_b64encode(cast(bytes, self.fill_value)).decode("ascii") + zarray_dict["fill_value"] = fill_value + _ = zarray_dict.pop("chunk_grid") zarray_dict["chunks"] = self.chunk_grid.chunk_shape diff --git a/tests/v3/test_v2.py b/tests/v3/test_v2.py index 95a5f66602..9c6d2cb8ee 100644 --- a/tests/v3/test_v2.py +++ b/tests/v3/test_v2.py @@ -6,6 +6,7 @@ from numcodecs.blosc import Blosc import zarr +import zarr.core.buffer.cpu from zarr import Array from zarr.storage import MemoryStore, StorePath @@ -46,3 +47,41 @@ def test_codec_pipeline() -> None: result = array[:] expected = np.ones(1) np.testing.assert_array_equal(result, expected) + + +async def test_v2_encode_decode(): + import json + + import zarr.core.buffer.cpu + import zarr.storage + + store = zarr.storage.MemoryStore(mode="w") + g = zarr.group(store=store, zarr_format=2) + g.create_array( + name="foo", + shape=(3,), + dtype="|S4", + fill_value=b"X", + ) + + result = await store.get("foo/.zarray", zarr.core.buffer.default_buffer_prototype()) + assert result is not None + + serialized = json.loads(result.to_bytes()) + expected = { + "chunks": [3], + # "compressor": {"blocksize": 0, "clevel": 5, "cname": "lz4", "id": "blosc", "shuffle": 1}, + "compressor": None, + "dtype": "|S4", + "fill_value": "WA==", + "filters": None, + "order": "C", + "shape": [3], + "zarr_format": 2, + "dimension_separator": ".", + } + assert serialized == expected + + data = zarr.open_array(store=store, path="foo")[:] + expected = np.full((3,), b"X", dtype="|S4") + np.testing.assert_equal(data, expected) From 2c71a8a9d55b0982880a7fdb031205cfb1caf6eb Mon Sep 17 00:00:00 2001 From: Tom Augspurger Date: Tue, 1 Oct 2024 11:42:09 -0500 Subject: [PATCH 2/5] cleanup --- src/zarr/core/metadata/v2.py | 2 +- tests/v3/test_v2.py | 7 ++----- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/src/zarr/core/metadata/v2.py b/src/zarr/core/metadata/v2.py index 16f5a09d3e..b4e1345331 100644 --- a/src/zarr/core/metadata/v2.py +++ b/src/zarr/core/metadata/v2.py @@ -163,7 +163,7 @@ def from_dict(cls, data: dict[str, Any]) -> ArrayV2Metadata: def to_dict(self) -> dict[str, JSON]: zarray_dict = super().to_dict() - if self.dtype.kind in "SV": + if self.dtype.kind in "SV" and self.fill_value is not None: # There's a relationship between self.dtype and self.fill_value # that mypy isn't aware of. The fact that we have S or V dtype here # means we should have a bytes-type fill_value. diff --git a/tests/v3/test_v2.py b/tests/v3/test_v2.py index 9c6d2cb8ee..3339a51949 100644 --- a/tests/v3/test_v2.py +++ b/tests/v3/test_v2.py @@ -1,3 +1,4 @@ +import json from collections.abc import Iterator import numpy as np @@ -7,6 +8,7 @@ import zarr import zarr.core.buffer.cpu +import zarr.storage from zarr import Array from zarr.storage import MemoryStore, StorePath @@ -50,11 +52,6 @@ def test_codec_pipeline() -> None: async def test_v2_encode_decode(): - import json - - import zarr.core.buffer.cpu - import zarr.storage - store = zarr.storage.MemoryStore(mode="w") g = zarr.group(store=store, zarr_format=2) g.create_array( From 3f2c29d5f436469b81ae78841d8e8b7b536b7ca8 Mon Sep 17 00:00:00 2001 From: Tom Augspurger Date: Wed, 2 Oct 2024 14:39:13 -0500 Subject: [PATCH 3/5] fixup --- src/zarr/core/metadata/v2.py | 2 +- tests/v3/test_v2.py | 11 ++++++----- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/zarr/core/metadata/v2.py b/src/zarr/core/metadata/v2.py index 5770952f40..d976653fa2 100644 --- a/src/zarr/core/metadata/v2.py +++ b/src/zarr/core/metadata/v2.py @@ -155,7 +155,7 @@ def from_dict(cls, data: dict[str, Any]) -> ArrayV2Metadata: # encoding and may work as fill values, e.g., the number 0 pass elif dtype.kind == "V": - fill_value = base64.standard_b64encode(fill_value_encoded) + fill_value = base64.standard_b64decode(fill_value_encoded) _data["fill_value"] = fill_value # zarr v2 allowed arbitrary keys here. diff --git a/tests/v3/test_v2.py b/tests/v3/test_v2.py index 3339a51949..74afd533f8 100644 --- a/tests/v3/test_v2.py +++ b/tests/v3/test_v2.py @@ -51,13 +51,15 @@ def test_codec_pipeline() -> None: np.testing.assert_array_equal(result, expected) -async def test_v2_encode_decode(): +@pytest.mark.parametrize("dtype", ["|S", "|V"]) +async def test_v2_encode_decode(dtype): store = zarr.storage.MemoryStore(mode="w") g = zarr.group(store=store, zarr_format=2) g.create_array( name="foo", shape=(3,), - dtype="|S4", + chunks=(3,), + dtype=dtype, fill_value=b"X", ) @@ -67,9 +69,8 @@ async def test_v2_encode_decode(): serialized = json.loads(result.to_bytes()) expected = { "chunks": [3], - # "compressor": {"blocksize": 0, "clevel": 5, "cname": "lz4", "id": "blosc", "shuffle": 1}, "compressor": None, - "dtype": "|S4", + "dtype": f"{dtype}0", "fill_value": "WA==", "filters": None, "order": "C", @@ -80,5 +81,5 @@ async def test_v2_encode_decode(): assert serialized == expected data = zarr.open_array(store=store, path="foo")[:] - expected = np.full((3,), b"X", dtype="|S4") + expected = np.full((3,), b"X", dtype=dtype) np.testing.assert_equal(data, expected) From 77e22e0d6905abed228e276254cbddf919da8afc Mon Sep 17 00:00:00 2001 From: Tom Augspurger Date: Fri, 4 Oct 2024 11:51:47 -0500 Subject: [PATCH 4/5] narrow exception types --- src/zarr/core/metadata/v2.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/zarr/core/metadata/v2.py b/src/zarr/core/metadata/v2.py index d976653fa2..937c80c6cd 100644 --- a/src/zarr/core/metadata/v2.py +++ b/src/zarr/core/metadata/v2.py @@ -1,6 +1,7 @@ from __future__ import annotations import base64 +import binascii from collections.abc import Iterable from enum import Enum from typing import TYPE_CHECKING, cast @@ -150,7 +151,7 @@ def from_dict(cls, data: dict[str, Any]) -> ArrayV2Metadata: try: fill_value = base64.standard_b64decode(fill_value_encoded) _data["fill_value"] = fill_value - except Exception: + except (TypeError, binascii.Error, ValueError): # be lenient, allow for other values that may have been used before base64 # encoding and may work as fill values, e.g., the number 0 pass From 22843a019f4152c6c0708d41dee3db885e5002e9 Mon Sep 17 00:00:00 2001 From: Tom Augspurger Date: Fri, 4 Oct 2024 12:01:19 -0500 Subject: [PATCH 5/5] Just reraise exceptions --- src/zarr/core/metadata/v2.py | 14 ++------------ tests/v3/test_v2.py | 1 + 2 files changed, 3 insertions(+), 12 deletions(-) diff --git a/src/zarr/core/metadata/v2.py b/src/zarr/core/metadata/v2.py index 937c80c6cd..ec44673f9d 100644 --- a/src/zarr/core/metadata/v2.py +++ b/src/zarr/core/metadata/v2.py @@ -1,7 +1,6 @@ from __future__ import annotations import base64 -import binascii from collections.abc import Iterable from enum import Enum from typing import TYPE_CHECKING, cast @@ -147,17 +146,8 @@ def from_dict(cls, data: dict[str, Any]) -> ArrayV2Metadata: if dtype.kind in "SV": fill_value_encoded = _data.get("fill_value") if fill_value_encoded is not None: - if dtype.kind == "S": - try: - fill_value = base64.standard_b64decode(fill_value_encoded) - _data["fill_value"] = fill_value - except (TypeError, binascii.Error, ValueError): - # be lenient, allow for other values that may have been used before base64 - # encoding and may work as fill values, e.g., the number 0 - pass - elif dtype.kind == "V": - fill_value = base64.standard_b64decode(fill_value_encoded) - _data["fill_value"] = fill_value + fill_value = base64.standard_b64decode(fill_value_encoded) + _data["fill_value"] = fill_value # zarr v2 allowed arbitrary keys here. # We don't want the ArrayV2Metadata constructor to fail just because someone put an diff --git a/tests/v3/test_v2.py b/tests/v3/test_v2.py index 74afd533f8..f488782d78 100644 --- a/tests/v3/test_v2.py +++ b/tests/v3/test_v2.py @@ -8,6 +8,7 @@ import zarr import zarr.core.buffer.cpu +import zarr.core.metadata import zarr.storage from zarr import Array from zarr.storage import MemoryStore, StorePath