Skip to content

Commit e62e45b

Browse files
vup903clauded-v-b
authored
Add unified JSON metadata validation via msgspec (#3285) (#4063)
* Add unified parse_json runtime type checker (#3285) Introduce zarr.core.json_parse.parse_json, a single type-annotation-driven validator that consolidates the scattered per-field parse_* helpers. Handles primitives, Literal, unions/Optional, fixed and variadic tuples, Sequence/list (coerced to tuple), Mapping/dict, and TypedDict, with a bool-vs-int safe primitive check. Adds tests/test_json_parse.py (94 tests) and a changelog fragment. No existing call sites migrated yet; this is the proof-of-direction module. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Migrate parse_order, parse_bool, parse_zarr_format to parse_json (#3285) Pilot migration delegating three representative helpers to the unified parse_json validator. Public signatures and return types are unchanged. parse_zarr_format re-wraps parse_json's ValueError/TypeError as MetadataValidationError with the original message to preserve observable behavior. parse_json is imported function-locally to avoid circular imports. Focused suites green: test_common/test_config/test_metadata/test_json_parse = 430 passed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Fix lint and make TypedDict NotRequired robust under future annotations (#3285) Apply ruff check/format to satisfy the Lint CI hook. Keep typing.Union/Optional spellings in tests (with noqa) to cover that origin path alongside X | Y. Rework _parse_typeddict to derive required/optional from get_type_hints(include_extras=True) + __total__ instead of __required_keys__, so class-syntax NotRequired is detected even when 'from __future__ import annotations' stringizes the hints (as in zarr's metadata modules). test_json_parse + test_common + test_metadata = 393 passed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Annotate origin as Any to satisfy mypy in _parse_typeddict (#3285) get_origin(hint) is Required/NotRequired tripped mypy's comparison-overlap and unreachable checks; typing origin as Any keeps the runtime check while satisfying mypy. Full 'uv run --frozen mypy' is clean (190 files); ruff check/format clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Migrate Batch 1 literal parsers to parse_json (#3285) Delegate the literal/type check of parse_indexing_order, parse_node_type, parse_node_type_array, parse_separator, two parse_zarr_format variants (group, v2), and parse_name to the unified parse_json. Public signatures and return types unchanged; each preserves its original exception type and message (wrapping parse_json's ValueError/TypeError into MetadataValidationError / NodeTypeValidationError / the original ValueError/TypeError where tests assert on them). parse_json imported function-locally to avoid circular imports. Focused suites + full mypy green (1196 passed). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Migrate Batch 2 codec primitive parsers to parse_json (#3285) Delegate the int/bool type check of parse_checksum, parse_clevel, parse_blocksize, parse_typesize, parse_gzip_level, and parse_zstd_level to parse_json, keeping each helper's range/bound check and exact error messages. Original exception types are preserved by wrapping parse_json's ValueError/TypeError. Note: parse_json rejects bool where the old isinstance(data, int) accepted it; verified no caller/test passes a bool to these, so this is a deliberate, more-correct strictening. parse_json imported function-locally. Codec suite + full mypy green (794 passed). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Replace hand-written parse_json with msgspec.convert (#3285) Delete the bespoke parse_json runtime type checker and route JSON metadata validation through msgspec.convert, which handles the type coercions zarr needs (Literal membership, int/bool strictness, list-to-tuple). A small hand-written fallback (validate_json_value) covers the recursive JSON values msgspec cannot build a schema for, and adds an explicit nesting-depth limit. The registry/dtype/numcodec parsers stay hand-written since they need runtime lookups. Also fixes a latent generator-exhaustion bug in parse_storage_transformers, adds msgspec as a dependency (pinned in the min_deps env), and preserves the exception types and messages every migrated helper raised. Net ~555 lines removed. Tests, mypy and ruff all green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Encapsulate convert + field-context error into parse_field (#3285) Address review feedback: factor the repeated convert-then-re-raise pattern out of each per-field parser. convert now raises a field-agnostic ValueError("Expected instance of TYPE, got DATA"); the new parse_field wraps it and re-raises with field context ("Failed to parse input for FIELD") using the caller's chosen exception type, chaining the original error. Every per-field parser collapses to a one-liner. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs: replace Sphinx roles in json_parse with plain literals (#3285) The new ci/lint_docs.py check flags :func:/:class: roles, which pass through as literal text under MkDocs/mkdocstrings instead of becoming links. msgspec is not in the configured inventories, so cross-references would not resolve; using inline literals matches how the codebase already writes np.nonzero. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix: keep literal members and the offending value in parse errors (#3285) The msgspec rewrite regressed error quality for Literal types: _type_name fell back to __name__, rendering Literal[3] as bare "Literal", and parse_field dropped the value entirely. The old per-field parsers reported both, e.g. "Invalid value for 'zarr_format'. Expected '3'. Got '3.0'." _type_name now renders parameterized types via str(), so members survive, and parse_field reports expected type and received value: Failed to parse input for 'zarr_format': expected Literal[3], got 3.0. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * refactor: hoist json_parse imports to module level (#3285) These were function-local to dodge import cycles under the old hand-written parse_json. After the msgspec rewrite json_parse's only zarr import is JSON under TYPE_CHECKING, so it has no runtime zarr dependency and cannot form a cycle. Verified each touched module still imports standalone. Hoisting exposed a latent packaging gap: msgspec was added to pyproject.toml but never locked, so uv.lock lacked it. That stayed hidden only because json_parse was imported lazily; at module level it broke `uv run --frozen` (ModuleNotFoundError in the docs job). Regenerated the lock, which adds msgspec 0.21.1 and nothing else. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs: note the stricter metadata parsing in the changelog (#3285) The old per-field checks compared with ==, so numerically equal values passed: zarr_format=2.0 was accepted. msgspec requires an actual int, so such inputs are now rejected. Spec-conforming metadata is unaffected. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs: link msgspec symbols via its inventory (#3285) Adds msgspec's Sphinx inventory so the json_parse docstrings can reference msgspec.convert and msgspec.ValidationError as real cross-references instead of inert literals. Verified the inventory is served at msgspec.dev (the jcristharif.com path in the package metadata is stale) and that both symbols resolve in it. Same-module names stay plain literals: zarr.core.json_parse is internal and not rendered in the API reference, so a cross-reference to it would not resolve and would fail `mkdocs build --strict`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: Davis Bennett <davis.v.bennett@gmail.com>
1 parent 9fd669f commit e62e45b

17 files changed

Lines changed: 369 additions & 73 deletions

File tree

changes/3285.feature.md

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
JSON metadata validation now delegates to ``msgspec.convert`` for the type
2+
coercions it supports (``Literal`` membership, ``int`` / ``bool`` strictness,
3+
list-to-tuple), replacing the per-field hand-written ``parse_*`` logic. A small
4+
fallback validates the recursive JSON values msgspec cannot, now with an
5+
explicit nesting-depth limit, and a latent generator-exhaustion bug in
6+
``parse_storage_transformers`` is fixed. See #3285.
7+
8+
As a result some metadata inputs are now parsed more strictly. The previous
9+
per-field checks compared values with ``==``, which accepts any numerically
10+
equal object, so a float such as ``2.0`` was accepted as ``zarr_format``; it is
11+
now rejected because it is not an ``int``. Booleans are likewise no longer
12+
accepted where an ``int`` is expected, since ``bool`` is an ``int`` subclass.
13+
Metadata that conforms to the Zarr specification is unaffected.

mkdocs.yml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -199,6 +199,7 @@ plugins:
199199
- https://docs.xarray.dev/en/stable/objects.inv
200200
- https://numpy.org/doc/stable/objects.inv
201201
- https://numcodecs.readthedocs.io/en/stable/objects.inv
202+
- https://msgspec.dev/objects.inv
202203
- https://developmentseed.org/obstore/latest/objects.inv
203204
- https://filesystem-spec.readthedocs.io/en/latest/objects.inv
204205
- https://requests.readthedocs.io/en/latest/objects.inv

pyproject.toml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ dependencies = [
4949
'google-crc32c>=1.5',
5050
'typing_extensions>=4.14',
5151
'donfig>=0.8',
52+
'msgspec>=0.19',
5253
]
5354

5455
dynamic = [
@@ -281,6 +282,7 @@ extra-dependencies = [
281282
'typing_extensions==4.14.*',
282283
'donfig==0.8.*',
283284
'obstore==0.5.*',
285+
'msgspec==0.19.*',
284286
]
285287

286288
[tool.hatch.envs.default]

src/zarr/codecs/blosc.py

Lines changed: 12 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
from zarr.core.buffer.cpu import as_numpy_array_wrapper
1515
from zarr.core.common import JSON, NamedRequiredConfig, parse_named_configuration
1616
from zarr.core.dtype.common import HasItemSize
17+
from zarr.core.json_parse import parse_field
1718

1819
if TYPE_CHECKING:
1920
from typing import Self
@@ -104,27 +105,24 @@ class BloscCname(metaclass=_DeprecatedStrEnumMeta):
104105

105106

106107
def parse_typesize(data: JSON) -> int:
107-
if isinstance(data, int):
108-
if data > 0:
109-
return data
110-
else:
111-
raise ValueError(
112-
f"Value must be greater than 0. Got {data}, which is less or equal to 0."
113-
)
114-
raise TypeError(f"Value must be an int. Got {type(data)} instead.")
108+
parsed: int = parse_field(data, int, "typesize", error=TypeError)
109+
if parsed > 0:
110+
return parsed
111+
else:
112+
raise ValueError(
113+
f"Value must be greater than 0. Got {parsed}, which is less or equal to 0."
114+
)
115115

116116

117117
# todo: real validation
118118
def parse_clevel(data: JSON) -> int:
119-
if isinstance(data, int):
120-
return data
121-
raise TypeError(f"Value should be an int. Got {type(data)} instead.")
119+
parsed: int = parse_field(data, int, "clevel", error=TypeError)
120+
return parsed
122121

123122

124123
def parse_blocksize(data: JSON) -> int:
125-
if isinstance(data, int):
126-
return data
127-
raise TypeError(f"Value should be an int. Got {type(data)} instead.")
124+
parsed: int = parse_field(data, int, "blocksize", error=TypeError)
125+
return parsed
128126

129127

130128
def _parse_cname(data: object) -> BloscCnameLiteral:

src/zarr/codecs/gzip.py

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
from zarr.abc.codec import BytesBytesCodec
1111
from zarr.core.buffer.cpu import as_numpy_array_wrapper
1212
from zarr.core.common import JSON, parse_named_configuration
13+
from zarr.core.json_parse import parse_field
1314

1415
if TYPE_CHECKING:
1516
from typing import Self
@@ -19,13 +20,12 @@
1920

2021

2122
def parse_gzip_level(data: JSON) -> int:
22-
if not isinstance(data, (int)):
23-
raise TypeError(f"Expected int, got {type(data)}")
24-
if data not in range(10):
23+
parsed: int = parse_field(data, int, "level", error=TypeError)
24+
if parsed not in range(10):
2525
raise ValueError(
26-
f"Expected an integer from the inclusive range (0, 9). Got {data} instead."
26+
f"Expected an integer from the inclusive range (0, 9). Got {parsed} instead."
2727
)
28-
return data
28+
return parsed
2929

3030

3131
@dataclass(frozen=True)

src/zarr/codecs/zstd.py

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
from zarr.abc.codec import BytesBytesCodec
1313
from zarr.core.buffer.cpu import as_numpy_array_wrapper
1414
from zarr.core.common import JSON, parse_named_configuration
15+
from zarr.core.json_parse import parse_field
1516

1617
if TYPE_CHECKING:
1718
from typing import Self
@@ -21,17 +22,15 @@
2122

2223

2324
def parse_zstd_level(data: JSON) -> int:
24-
if isinstance(data, int):
25-
if data >= 23:
26-
raise ValueError(f"Value must be less than or equal to 22. Got {data} instead.")
27-
return data
28-
raise TypeError(f"Got value with type {type(data)}, but expected an int.")
25+
parsed: int = parse_field(data, int, "level", error=TypeError)
26+
if parsed >= 23:
27+
raise ValueError(f"Value must be less than or equal to 22. Got {parsed} instead.")
28+
return parsed
2929

3030

3131
def parse_checksum(data: JSON) -> bool:
32-
if isinstance(data, bool):
33-
return data
34-
raise TypeError(f"Expected bool. Got {type(data)}.")
32+
parsed: bool = parse_field(data, bool, "checksum", error=TypeError)
33+
return parsed
3534

3635

3736
@dataclass(frozen=True)

src/zarr/core/chunk_key_encodings.py

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,15 +13,14 @@
1313
NamedConfig,
1414
parse_named_configuration,
1515
)
16+
from zarr.core.json_parse import parse_field
1617
from zarr.registry import get_chunk_key_encoding_class, register_chunk_key_encoding
1718

1819
SeparatorLiteral = Literal[".", "/"]
1920

2021

2122
def parse_separator(data: JSON) -> SeparatorLiteral:
22-
if data not in (".", "/"):
23-
raise ValueError(f"Expected an '.' or '/' separator. Got {data} instead.")
24-
return cast("SeparatorLiteral", data)
23+
return cast("SeparatorLiteral", parse_field(data, Literal[".", "/"], "separator"))
2524

2625

2726
class ChunkKeyEncodingParams(TypedDict):

src/zarr/core/common.py

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
from typing_extensions import ReadOnly
2121

2222
from zarr.core.config import config as zarr_config
23+
from zarr.core.json_parse import convert, parse_field
2324
from zarr.errors import ZarrRuntimeWarning
2425

2526
if TYPE_CHECKING:
@@ -147,12 +148,13 @@ def parse_enum[E: Enum](data: object, cls: type[E]) -> E:
147148

148149

149150
def parse_name(data: JSON, expected: str | None = None) -> str:
150-
if isinstance(data, str):
151-
if expected is None or data == expected:
152-
return data
153-
raise ValueError(f"Expected '{expected}'. Got {data} instead.")
154-
else:
155-
raise TypeError(f"Expected a string, got an instance of {type(data)}.")
151+
try:
152+
data = cast("str", convert(data, str))
153+
except (ValueError, TypeError) as exc:
154+
raise TypeError(f"Expected a string, got an instance of {type(data)}.") from exc
155+
if expected is None or data == expected:
156+
return data
157+
raise ValueError(f"Expected '{expected}'. Got {data} instead.")
156158

157159

158160
def parse_configuration(data: JSON) -> JSON:
@@ -227,15 +229,11 @@ def parse_fill_value(data: Any) -> Any:
227229

228230

229231
def parse_order(data: Any) -> Literal["C", "F"]:
230-
if data in ("C", "F"):
231-
return cast("Literal['C', 'F']", data)
232-
raise ValueError(f"Expected one of ('C', 'F'), got {data} instead.")
232+
return cast("Literal['C', 'F']", parse_field(data, Literal["C", "F"], "order"))
233233

234234

235235
def parse_bool(data: Any) -> bool:
236-
if isinstance(data, bool):
237-
return data
238-
raise ValueError(f"Expected bool, got {data} instead.")
236+
return cast("bool", convert(data, bool))
239237

240238

241239
def parse_int(data: Any) -> int:

src/zarr/core/config.py

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,8 @@
3333

3434
from donfig import Config as DConfig
3535

36+
from zarr.core.json_parse import parse_field
37+
3638
if TYPE_CHECKING:
3739
from donfig.config_obj import ConfigSet
3840

@@ -159,7 +161,4 @@ def enable_gpu(self) -> ConfigSet:
159161

160162

161163
def parse_indexing_order(data: Any) -> Literal["C", "F"]:
162-
if data in ("C", "F"):
163-
return cast("Literal['C', 'F']", data)
164-
msg = f"Expected one of ('C', 'F'), got {data} instead."
165-
raise ValueError(msg)
164+
return cast("Literal['C', 'F']", parse_field(data, Literal["C", "F"], "order"))

src/zarr/core/group.py

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@
4646
)
4747
from zarr.core.config import config
4848
from zarr.core.dtype import parse_data_type
49+
from zarr.core.json_parse import parse_field
4950
from zarr.core.metadata import ArrayV2Metadata, ArrayV3Metadata
5051
from zarr.core.metadata.io import save_metadata
5152
from zarr.core.sync import SyncMixin, sync
@@ -85,18 +86,15 @@
8586

8687
def parse_zarr_format(data: Any) -> ZarrFormat:
8788
"""Parse the zarr_format field from metadata."""
88-
if data in (2, 3):
89-
return cast("ZarrFormat", data)
90-
msg = f"Invalid zarr_format. Expected one of 2 or 3. Got {data}."
91-
raise ValueError(msg)
89+
return cast("ZarrFormat", parse_field(data, Literal[2, 3], "zarr_format"))
9290

9391

9492
def parse_node_type(data: Any) -> NodeType:
9593
"""Parse the node_type field from metadata."""
96-
if data in ("array", "group"):
97-
return cast("Literal['array', 'group']", data)
98-
msg = f"Invalid value for 'node_type'. Expected 'array' or 'group'. Got '{data}'."
99-
raise MetadataValidationError(msg)
94+
return cast(
95+
"Literal['array', 'group']",
96+
parse_field(data, Literal["array", "group"], "node_type", error=MetadataValidationError),
97+
)
10098

10199

102100
# todo: convert None to empty dict

0 commit comments

Comments
 (0)