feat(zarr-metadata): model-layer groundwork (layer 0)

packages/zarr-metadata/changes/4420.bugfix.md

@@ -0,0 +1,33 @@
+Attributes are user data, and a non-finite number in them is read,
+validated and written back. zarr-python writes attributes with the
+defaults of Python's `json` module, so an xarray `_FillValue` or a CF
+`missing_value` of NaN is stored as a bare `NaN`, which RFC 8259 lacks;
+the model refused such a document and could not read the store at all.
+A node's `attributes`, a v2 `.zattrs`, the `.zattrs` entries of a v2
+`.zmetadata`, and the attributes of the nodes a v3 group's inline
+`consolidated_metadata` holds may now hold `NaN`, `Infinity` and
+`-Infinity`, in every document `validate_*`, `parse_*` and `is_*`, and in
+`from_key_value` and `to_key_value`. Wherever the spec interprets a value
+it spells those numbers as strings, so anywhere else a non-finite number
+is still refused, and the store reader now says where one is instead of
+failing to decode the document. `is_json`, `validate_json` and
+`parse_json` judge by RFC 8259 alone, so a document the node guards
+accept can hold a value `is_json` refuses.
+
+`to_key_value` validates the document it writes as `from_key_value`
+validates what it reads, and raises `MetadataValidationError` with every
+problem instead of writing a document the reader refuses. A model built
+by hand is not validated, so the writer is where these are caught:
+
+- a non-finite number outside attributes, which was refused before too;
+- a non-string attribute key, which was written as a string;
+- a value that is not JSON, which raised `TypeError`;
+- anything else the reader refuses, such as `dimension_names` longer than
+  `shape`, which was written and then refused on read.
+
+Any sequence is written as an array. A number too large for a float, such
+as `1e400`, is read as infinity, so in attributes it is now accepted and
+written back as `Infinity`.
+
+Through `zarr_metadata.pydantic`, pydantic writes a non-finite attribute
+as `null` unless the enclosing model sets `ser_json_inf_nan="constants"`.

packages/zarr-metadata/changes/4421.bugfix.md

@@ -0,0 +1,4 @@
+`from_key_value` takes a mapping keyed by `str` or by any narrower key
+type, so reading back what `to_key_value` wrote type-checks. `to_key_value`
+keys its result by the store-key literals, and since a mapping's key type is
+invariant, a `Mapping[str, bytes]` parameter refused it.

packages/zarr-metadata/changes/4422.bugfix.md

@@ -0,0 +1,3 @@
+`MetadataValidationError` pickles and copies with its message. A copy was
+built from the error's message, and its message came out as the
+original's characters, one per line.

packages/zarr-metadata/changes/4422.feature.md

@@ -0,0 +1,7 @@
+**Breaking:** `ValidationProblem` and `MetadataValidationError` check at run
+time what their annotations declare, and raise `TypeError` otherwise. A
+problem's `loc` is a tuple of strings and integers, so a list, which was
+accepted before, is now refused, as is a `bool`; its `message` is a string
+and its `kind` one of the kinds; an error holds problems. A caller that is
+not type-checked fails where it builds the value, not later, where a
+location is read.

packages/zarr-metadata/justfile

@@ -8,26 +8,55 @@ set positional-arguments
 default:
     @just --list
 
-# Run the test suite; extra args are passed to pytest
+# Run the test suite on the project interpreter; extra args are passed to pytest
 test *args:
     uv run --group test pytest tests "$@"
 
+# The oldest and newest interpreters the package is declared for: the floor is
+# `requires-python`, the ceiling the newest classifier, both read from
+# pyproject.toml. CI's matrix is written out in its workflow, so a change to
+# the declared range is made there too.
+# uv fetches an interpreter it does not have. A difference in how a version
+# materialises annotations or resolves a stub shows up here, not only in CI.
+# Run the test suite on the oldest and newest supported interpreters
+test-versions *args:
+    #!/usr/bin/env bash
+    set -euo pipefail
+    read -r floor ceiling < <(uv run python - <<'EOF'
+    import re
+    import tomllib
+
+    project = tomllib.load(open("pyproject.toml", "rb"))["project"]
+    floor = re.fullmatch(r">=\s*(\d+\.\d+)", project["requires-python"]).group(1)
+    declared = [
+        classifier.rsplit(" ", 1)[1]
+        for classifier in project["classifiers"]
+        if classifier.startswith("Programming Language :: Python :: 3.")
+    ]
+    print(floor, max(declared, key=lambda version: tuple(map(int, version.split(".")))))
+    EOF
+    )
+    for version in "$floor" "$ceiling"; do
+        echo "== python $version =="
+        uv run --python "$version" --group test pytest tests "$@"
+    done
+
 # Lint with the same invocation CI uses
 lint:
     uvx ruff check .
 
-# Pinned to the last pyright that types PEP 661 sentinels in class attributes
-# correctly; 1.1.405+ regressed (microsoft/pyright#11115). Unpin when fixed.
-pyright_version := "1.1.404"
+# Pinned so a pyright release cannot turn CI red on its own schedule. Bump it
+# deliberately: run the new version, read what it found, then move the pin.
+pyright_version := "1.1.414"
 
-# CI runs pyright on python 3.11; the pinned pyright predates 3.14, whose
-# stdlib it cannot parse, so pin the interpreter to match CI.
-# Type-check the package sources
+# Run under the interpreter CI runs pyright under, so a version-dependent
+# stdlib or stub difference shows up here rather than only in CI.
+# Type-check the package, sources and tests alike
 typecheck:
-    uv run --python 3.11 --group test --with 'pyright=={{ pyright_version }}' pyright src
+    uv run --python 3.11 --group test --with 'pyright=={{ pyright_version }}' pyright
 
-# Run everything CI runs for this package
-check: lint typecheck test docs-check
+# Run everything CI runs for this package, on both ends of the version range
+check: lint typecheck test-versions docs-check
 
 # Preview the changelog that the next release would generate
 changelog-draft:

packages/zarr-metadata/pyproject.toml

@@ -135,15 +135,42 @@ checks = [
     "PR06",
 ]
 
-# CI pins pyright==1.1.404: later versions regress PEP 661 sentinel typing in
-# class attributes (microsoft/pyright#11115), which zarr_metadata.model._sentinel
-# relies on. Use the same pin locally; unpin when the fix lands.
+# The pyright version lives in the justfile, which is what CI runs; pinning it
+# keeps a pyright release from turning CI red on its own schedule.
 [tool.pyright]
-include = ["src"]
+include = ["src", "tests"]
+# Pyright is the only checker that runs on this package (the root mypy
+# config covers the root package alone), and it honours `# type: ignore`
+# as a blanket suppression of every diagnostic on the line. So that
+# spelling is switched off: a suppression is `# pyright: ignore[rule]`,
+# names the rule, and is an error once it stops suppressing anything.
+enableTypeIgnoreComments = false
+reportUnnecessaryTypeIgnoreComment = "error"
+# `tests` is a package imported as `tests.*`, which pytest resolves from the
+# rootdir; pyright needs the same root on its search path.
+extraPaths = ["."]
 enableExperimentalFeatures = true
 typeCheckingMode = "strict"
 pythonVersion = "3.11"
 
+# Tests are checked, but not for the inference strictness the sources are held
+# to: test bodies are full of untyped JSON literals whose inferred types are of
+# no interest, and strict mode reports about ninety of those. What is kept is
+# everything a rename or a signature change breaks -- a dangling import, a call
+# that no longer type-checks, an argument of the wrong type -- which is the
+# reason for checking tests at all. Tests also reach into private modules and
+# define functions purely to be refused at registration, both deliberately.
+[[tool.pyright.executionEnvironments]]
+root = "tests"
+reportMissingTypeArgument = false
+reportPrivateUsage = false
+reportUnknownArgumentType = false
+reportUnknownLambdaType = false
+reportUnknownMemberType = false
+reportUnknownParameterType = false
+reportUnknownVariableType = false
+reportUnusedFunction = false
+
 [tool.towncrier]
 # Fragments for this package live alongside the package source, separate
 # from the parent zarr-python `changes/` directory, so a PR touching only

packages/zarr-metadata/src/zarr_metadata/model/_array.py

@@ -14,6 +14,7 @@
 from zarr_metadata.model._validation import (
     ARRAY_METADATA_STANDARD_KEYS_V3,
     MetadataValidationError,
+    StoreKey,
     ValidationProblem,
     arrays_to_tuples,
     dump_store_json,
@@ -322,13 +323,16 @@ def must_understand_fields(self) -> dict[str, ZarrV3ExtensionField]:
         return must_understand_subset(self.extra_fields)
 
     @classmethod
-    def from_key_value(cls, mapping: Mapping[str, bytes]) -> ZarrV3ArrayMetadata:
+    def from_key_value(cls, mapping: Mapping[StoreKey, bytes]) -> ZarrV3ArrayMetadata:
         return cls.from_json(load_store_json(mapping, ZARR_V3_ARRAY_METADATA_STORE_KEY))
 
     def to_key_value(
         self, *, indent: int | str | None = None
     ) -> Mapping[ZarrV3ArrayMetadataStoreKey, bytes]:
-        return {ZARR_V3_ARRAY_METADATA_STORE_KEY: dump_store_json(self.to_json(), indent=indent)}
+        # A model built by hand is not validated: its document is written only
+        # if it reads as `from_json` reads one, and every problem is raised.
+        document = parse_array_metadata_v3(self.to_json())
+        return {ZARR_V3_ARRAY_METADATA_STORE_KEY: dump_store_json(document, indent=indent)}
 
 
 class ZarrV2ArrayMetadataPartial(TypedDict, total=False):
@@ -468,7 +472,7 @@ def from_json(cls, data: object) -> ZarrV2ArrayMetadata:
         )
 
     @classmethod
-    def from_key_value(cls, mapping: Mapping[str, bytes]) -> ZarrV2ArrayMetadata:
+    def from_key_value(cls, mapping: Mapping[StoreKey, bytes]) -> ZarrV2ArrayMetadata:
         zarray_raw = load_store_json(mapping, ZARR_V2_ARRAY_METADATA_STORE_KEY)
         if not isinstance(zarray_raw, Mapping):
             return cls.from_json(zarray_raw)
@@ -493,11 +497,16 @@ def to_key_value(
     ) -> Mapping[ZarrV2ArrayMetadataStoreKey | ZarrV2AttributesStoreKey, bytes]:
         # Attributes live only in the sibling `.zattrs` file; the `.zarray`
         # document must exclude them. The `.zattrs` key is present exactly
-        # when attributes are set (even empty) — UNSET emits no file.
-        zarray = {k: v for k, v in self.to_json().items() if k != "attributes"}
+        # when attributes are set (even empty) — UNSET emits no file. A model
+        # built by hand is not validated: its document is written only if it
+        # reads as `from_json` reads one, and every problem is raised.
+        document = parse_array_metadata_v2(self.to_json())
+        zarray = {k: v for k, v in document.items() if k != "attributes"}
         out: dict[ZarrV2ArrayMetadataStoreKey | ZarrV2AttributesStoreKey, bytes] = {
             ZARR_V2_ARRAY_METADATA_STORE_KEY: dump_store_json(zarray, indent=indent)
         }
-        if self.attributes is not UNSET:
-            out[ZARR_V2_ATTRIBUTES_STORE_KEY] = dump_store_json(self.attributes, indent=indent)
+        if "attributes" in document:
+            out[ZARR_V2_ATTRIBUTES_STORE_KEY] = dump_store_json(
+                document["attributes"], indent=indent
+            )
         return out

packages/zarr-metadata/src/zarr_metadata/model/_group.py

@@ -18,14 +18,16 @@
 from zarr_metadata.model._validation import (
     GROUP_METADATA_STANDARD_KEYS_V3,
     MetadataValidationError,
+    StoreKey,
     ValidationProblem,
     arrays_to_tuples,
     dump_store_json,
     load_store_json,
     parse_group_metadata_v2,
     parse_group_metadata_v3,
+    refine_json,
+    refine_user_data,
     validate_consolidated_metadata_v3,
-    validate_json,
 )
 from zarr_metadata.v2.attributes import ZARR_V2_ATTRIBUTES_STORE_KEY
 from zarr_metadata.v2.consolidated import ZARR_V2_CONSOLIDATED_METADATA_STORE_KEY
@@ -176,13 +178,16 @@ def must_understand_fields(self) -> dict[str, ZarrV3ExtensionField]:
         return must_understand_subset(self.extra_fields)
 
     @classmethod
-    def from_key_value(cls, mapping: Mapping[str, bytes]) -> ZarrV3GroupMetadata:
+    def from_key_value(cls, mapping: Mapping[StoreKey, bytes]) -> ZarrV3GroupMetadata:
         return cls.from_json(load_store_json(mapping, ZARR_V3_GROUP_METADATA_STORE_KEY))
 
     def to_key_value(
         self, *, indent: int | str | None = None
     ) -> Mapping[ZarrV3GroupMetadataStoreKey, bytes]:
-        return {ZARR_V3_GROUP_METADATA_STORE_KEY: dump_store_json(self.to_json(), indent=indent)}
+        # A model built by hand is not validated: its document is written only
+        # if it reads as `from_json` reads one, and every problem is raised.
+        document = parse_group_metadata_v3(self.to_json())
+        return {ZARR_V3_GROUP_METADATA_STORE_KEY: dump_store_json(document, indent=indent)}
 
 
 @dataclass(frozen=True, slots=True, kw_only=True)
@@ -314,7 +319,7 @@ def from_json(cls, data: object) -> ZarrV2GroupMetadata:
         return cls(attributes=(dict(parsed["attributes"]) if "attributes" in parsed else UNSET))
 
     @classmethod
-    def from_key_value(cls, mapping: Mapping[str, bytes]) -> ZarrV2GroupMetadata:
+    def from_key_value(cls, mapping: Mapping[StoreKey, bytes]) -> ZarrV2GroupMetadata:
         zgroup_raw = load_store_json(mapping, ZARR_V2_GROUP_METADATA_STORE_KEY)
         if not isinstance(zgroup_raw, Mapping):
             return cls.from_json(zgroup_raw)
@@ -339,13 +344,18 @@ def to_key_value(
     ) -> Mapping[ZarrV2GroupMetadataStoreKey | ZarrV2AttributesStoreKey, bytes]:
         # Attributes live only in the sibling `.zattrs` file; the `.zgroup`
         # document must exclude them. The `.zattrs` key is present exactly
-        # when attributes are set (even empty) — UNSET emits no file.
-        zgroup = {k: v for k, v in self.to_json().items() if k != "attributes"}
+        # when attributes are set (even empty) — UNSET emits no file. A model
+        # built by hand is not validated: its document is written only if it
+        # reads as `from_json` reads one, and every problem is raised.
+        document = parse_group_metadata_v2(self.to_json())
+        zgroup = {k: v for k, v in document.items() if k != "attributes"}
         out: dict[ZarrV2GroupMetadataStoreKey | ZarrV2AttributesStoreKey, bytes] = {
             ZARR_V2_GROUP_METADATA_STORE_KEY: dump_store_json(zgroup, indent=indent)
         }
-        if self.attributes is not UNSET:
-            out[ZARR_V2_ATTRIBUTES_STORE_KEY] = dump_store_json(self.attributes, indent=indent)
+        if "attributes" in document:
+            out[ZARR_V2_ATTRIBUTES_STORE_KEY] = dump_store_json(
+                document["attributes"], indent=indent
+            )
         return out
 
 
@@ -377,15 +387,18 @@ def from_json(cls, data: object) -> ZarrV2ConsolidatedMetadata:
             raise MetadataValidationError(
                 [ValidationProblem((), "expected a mapping", "invalid_type")]
             )
-        doc = cast("Mapping[str, object]", normalized)
+        doc = cast("Mapping[object, object]", normalized)
         problems: list[ValidationProblem] = [
             ValidationProblem((key,), "missing required key", "missing_key")
             for key in ("zarr_consolidated_format", "metadata")
             if key not in doc
         ]
         problems.extend(
             ValidationProblem((key,), "unexpected document member", "invalid_value")
-            for key in doc.keys() - {"zarr_consolidated_format", "metadata"}
+            if isinstance(key, str)
+            else ValidationProblem((), f"non-string document key {key!r}", "invalid_type")
+            for key in doc
+            if key not in {"zarr_consolidated_format", "metadata"}
         )
         if "zarr_consolidated_format" in doc and (
             not isinstance(doc["zarr_consolidated_format"], int)
@@ -399,6 +412,7 @@ def from_json(cls, data: object) -> ZarrV2ConsolidatedMetadata:
                     "invalid_value",
                 )
             )
+        refined: dict[str, JSONValue] = {}
         if "metadata" in doc:
             entries = doc["metadata"]
             if not isinstance(entries, Mapping) or not all(
@@ -410,28 +424,29 @@ def from_json(cls, data: object) -> ZarrV2ConsolidatedMetadata:
                     )
                 )
             else:
+                # Each entry is the document its key names: a `.zattrs` is
+                # user data, and any other is JSON by RFC 8259.
                 for key, value in cast("Mapping[str, object]", entries).items():
-                    problems.extend(
-                        ValidationProblem(
-                            ("metadata", key, *problem.loc), problem.message, problem.kind
-                        )
-                        for problem in validate_json(value)
+                    refine = (
+                        refine_user_data
+                        if key.rsplit("/", 1)[-1] == ZARR_V2_ATTRIBUTES_STORE_KEY
+                        else refine_json
                     )
+                    entry, found = refine(value, ("metadata", key))
+                    problems.extend(found)
+                    refined[key] = entry
         if len(problems) != 0:
             raise MetadataValidationError(problems)
-        entries_tupled = cast(
-            "dict[str, JSONValue]",
-            arrays_to_tuples(dict(cast("Mapping[str, object]", doc["metadata"]))),
-        )
-        return cls(metadata=entries_tupled)
+        return cls(metadata=refined)
 
     @classmethod
-    def from_key_value(cls, mapping: Mapping[str, bytes]) -> ZarrV2ConsolidatedMetadata:
+    def from_key_value(cls, mapping: Mapping[StoreKey, bytes]) -> ZarrV2ConsolidatedMetadata:
         return cls.from_json(load_store_json(mapping, ZARR_V2_CONSOLIDATED_METADATA_STORE_KEY))
 
     def to_key_value(
         self, *, indent: int | str | None = None
     ) -> Mapping[ZarrV2ConsolidatedMetadataStoreKey, bytes]:
-        return {
-            ZARR_V2_CONSOLIDATED_METADATA_STORE_KEY: dump_store_json(self.to_json(), indent=indent)
-        }
+        # A model built by hand is not validated: it is written only as
+        # `from_json` reads it, and every problem is raised.
+        document = ZarrV2ConsolidatedMetadata.from_json(self.to_json()).to_json()
+        return {ZARR_V2_CONSOLIDATED_METADATA_STORE_KEY: dump_store_json(document, indent=indent)}