Add unified parse_json runtime type checker (#3285) [draft / proof-of-direction]

[vup903]

Summary

This is a draft / proof-of-direction PR for #3285, opened to show the intended approach and get early feedback before migrating all call sites — per @d-v-b's request in the issue thread.

It introduces a single unified runtime type checker, parse_json(value, type_annotation), that consolidates the scattered per-field parse_* helpers (there are currently ~42 of them across src/zarr). The function validates a JSON-decoded value against a type annotation and returns data assignable to that annotation (coercing where sensible, e.g. parse_json([1, 2, 3], tuple[int, int, int]) -> (1, 2, 3)), or raises a clear error.

What's in this draft

• src/zarr/core/json_parse.py — parse_json plus sub-parsers, scoped to the JSON-relevant categories named in the issue:
  • primitives: None, str, int, float, bool (with a bool-vs-int-safe check: True is not accepted for int, and 1 is not accepted for bool)
  • Literal[...]
  • unions / Optional
  • fixed and variadic tuple
  • Sequence[T] / list[T] → returned as a tuple
  • Mapping[str, T] / dict[str, T]
  • TypedDict
• tests/test_json_parse.py — 94 tests covering every category, incl. the bool/int edge cases, coercion, unions, nested TypedDict.
• Pilot migration of three representative helpers to delegate to parse_json, with public signatures and behavior unchanged:
  • core/common.py::parse_order → parse_json(data, Literal["C", "F"])
  • core/common.py::parse_bool → parse_json(data, bool)
  • core/metadata/v3.py::parse_zarr_format → parse_json(data, Literal[3]), re-wrapping the error as MetadataValidationError to preserve the original exception type and message.

Focused suites pass locally (Python 3.12, test.py3.12-optional env): test_common, test_config, test_metadata, and test_json_parse → 430 passed.

Open questions for maintainers (direction feedback)

1. Module name/location — src/zarr/core/json_parse.py is a placeholder; happy to move it (e.g. into a typing/metadata utility module).
2. Exceptions — parse_json raises plain ValueError / TypeError since it's field-agnostic; field-specific callers (like parse_zarr_format) re-wrap into their domain error. Does that split match what you'd want?
3. TypedDict + NotRequired — the current _parse_typeddict reads __required_keys__ / __optional_keys__ directly. Under from __future__ import annotations with class-syntax TypedDicts, typing_extensions can't see the NotRequired wrapper at class-creation time, so an optional key can be mis-classified as required. The robust fix is to resolve hints via typing_extensions.get_type_hints(..., include_extras=True) and inspect for Required/NotRequired. I've left this for the follow-up once the overall direction is confirmed, since none of the three pilot helpers are TypedDicts.

If the direction looks right, I'll migrate the remaining parse_* call sites incrementally in follow-up commits.

Closes #3285 once fully migrated (draft for now).

[codecov]

Codecov Report

❌ Patch coverage is 88.73239% with 24 lines in your changes missing coverage. Please review.
✅ Project coverage is 93.49%. Comparing base (22818d9) to head (6280ac8).

Files with missing lines	Patch %	Lines
src/zarr/codecs/blosc.py	65.00%	7 Missing ⚠️
src/zarr/codecs/zstd.py	64.28%	5 Missing ⚠️
src/zarr/core/json_parse.py	95.93%	5 Missing ⚠️
src/zarr/core/group.py	75.00%	3 Missing ⚠️
src/zarr/codecs/gzip.py	71.42%	2 Missing ⚠️
src/zarr/core/chunk_key_encodings.py	60.00%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4063      +/-   ##
==========================================
- Coverage   93.50%   93.49%   -0.01%     
==========================================
  Files          90       91       +1     
  Lines       11979    12132     +153     
==========================================
+ Hits        11201    11343     +142     
- Misses        778      789      +11     

Files with missing lines	Coverage Δ
src/zarr/core/common.py	89.94% <100.00%> (+1.25%)	⬆️
src/zarr/core/config.py	100.00% <100.00%> (ø)
src/zarr/core/metadata/v2.py	89.61% <100.00%> (+0.17%)	⬆️
src/zarr/core/metadata/v3.py	94.01% <100.00%> (+0.06%)	⬆️
src/zarr/codecs/gzip.py	90.90% <71.42%> (-1.78%)	⬇️
src/zarr/core/chunk_key_encodings.py	90.00% <60.00%> (-1.18%)	⬇️
src/zarr/core/group.py	95.11% <75.00%> (-0.09%)	⬇️
src/zarr/codecs/zstd.py	88.33% <64.28%> (-2.41%)	⬇️
src/zarr/core/json_parse.py	95.93% <95.93%> (ø)
src/zarr/codecs/blosc.py	93.43% <65.00%> (-1.89%)	⬇️

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[vup903]

Hi @d-v-b, I put up a draft so you can see where I'm heading before I take it further.

So far it adds src/zarr/core/json_parse.py with a single parse_json(value, type_annotation) function. It validates a JSON-decoded value against a type annotation and returns assignable data (coercing sequences to tuples), or raises a clear error. I kept the scope to the JSON types listed in the issue: primitives, Literal, unions/Optional, tuple, Sequence/list, Mapping/dict, and TypedDict. There's a test file (tests/test_json_parse.py) with 94 cases covering each category, including the bool/int edge case and NotRequired under from __future__ import annotations.

On the de-dup side, I've moved 16 of the scattered parse_* helpers over to parse_json already. I split it into three small batches to keep each one easy to review: the pilot (parse_order, parse_bool, parse_zarr_format), the literal ones (parse_indexing_order, parse_node_type, parse_node_type_array, parse_separator, the zarr_format variants, parse_name), and the codec primitives (parse_clevel, parse_blocksize, parse_typesize, parse_gzip_level, parse_zstd_level, parse_checksum). Public signatures and the observable behavior (exception type and message) stay the same in every case.

Before I migrate the rest, a few things I'd like your read on:

1. Is src/zarr/core/json_parse.py a reasonable home for this, or would you rather it live somewhere else?
2. parse_json raises plain ValueError/TypeError, and the field-specific callers re-wrap into their own error (like MetadataValidationError). Does that split feel right to you?
3. For TypedDict, I resolve hints with get_type_hints(..., include_extras=True) and read Required/NotRequired plus __total__ instead of __required_keys__, so a class-syntax NotRequired still works when annotations are stringized. Open to doing it differently if you have a preference.
4. parse_json(x, int) rejects bool (since bool is an int subclass), where the old isinstance check let it through. I went with the stricter behavior because nothing relied on passing a bool there, but happy to revert if you'd rather keep it loose.

I'm holding off on the sequence/mapping/union helpers that dispatch to registries or factory methods until I hear back on the above.

One heads up on CI: the failing Test jobs are a pre-existing collection error in tests/test_group.py (duplicate parametrization of 'store') and tests/test_docs.py under the newer pytest. It's already on main and isn't from this change. Lint and pre-commit.ci are passing.

Let me know what you think on the four questions and I'll keep going.

[chuckwondo]

@vup903, if we are willing to add msgspec as a (very small) dependency, I believe we can eliminate the vast majority of this code, as msgspec.convert handles nearly all (if not all) of this logic. I might be off, as I have not thoroughly looked through the entirety of the affected code, but I'd like to hear from @d-v-b on this point.

[d-v-b]

nearly all of the types we need are now defined in zarr-metadata, so we should investigate if msgspec can handle all of those types correctly. zarr-metadata itself uses pydantic for its test suite, and I think i ended up choosing that after tryng msgspec and running into some issues, but I don't recall what they were. Maybe something to do with tuples? If its helpful, we could widen the JSON collection types in zarr-metadata to Sequence instead of tuple.

[vup903]

Thanks @chuckwondo, that's a good idea. I spent some time poking at msgspec.convert against these types to see how far it gets, and wanted to share what I found (msgspec 0.21.1, Python 3.12).

The good news is that most of the categories work out of the box, including the one you were worried about, @d-v-b:

• tuple coercion is fine now. msgspec.convert([1, 2, 3], tuple[int, ...]) returns (1, 2, 3), fixed-length tuples work, tuple[str | None, ...] works, and a wrong-length array is rejected. So whatever the old tuple issue was, I couldn't reproduce it on the current version.
• bool vs int is strict in the way we want: True is rejected for int and 1 is rejected for bool.
• Literal, Mapping[str, T], and TypedDict with NotRequired all behave correctly, including under from __future__ import annotations.

The two things that don't work are both load-bearing for the real zarr-metadata types, though:

1. The recursive JSONValue alias. msgspec.convert(doc, ArrayMetadataV3) fails with TypeError: Type 'JSONValue' is not supported. JSONValue is a recursive TypeAliasType (int | float | bool | None | str | list[JSONValue] | tuple[JSONValue, ...] | Mapping[str, JSONValue]), and msgspec doesn't accept it. Since fill_value, attributes, and the various config mappings are all typed as JSONValue, this blocks converting the actual array/group documents.

2. extra_items= (PEP 728). msgspec ignores it. With a TypedDict(..., extra_items=int), converting {"name": "x", "k": 5} silently drops k, and it doesn't even flag {"name": "x", "k": "not_an_int"} as invalid. ArrayMetadataV3 and GroupMetadataV3 use extra_items=ExtensionFieldV3 to validate v3 extension fields, so this matters for spec-correct validation of extension keys.

So my read is that msgspec handles the leaf/primitive/tuple cases really well, but it can't currently validate the two zarr-specific constructs (JSONValue and extra_items=) end to end. A couple of directions from here:

• If you'd be open to it, we could keep a thin hand-written layer just for the parts msgspec can't do (JSONValue and extra_items=) and lean on msgspec.convert for everything else. That would still delete most of the per-field parse_* code.
• Or, if widening the zarr-metadata collection types to Sequence (as you mentioned) also comes with dropping extra_items= / reshaping JSONValue into something msgspec understands, msgspec might cover the whole surface.

Happy to prototype whichever direction you prefer. What's your instinct here?

[d-v-b]

The recursive JSONValue alias. msgspec.convert(doc, ArrayMetadataV3) fails with TypeError: Type 'JSONValue' is not supported. JSONValue is a recursive TypeAliasType (int | float | bool | None | str | list[JSONValue] | tuple[JSONValue, ...] | Mapping[str, JSONValue]), and msgspec doesn't accept it. Since fill_value, attributes, and the various config mappings are all typed as JSONValue, this blocks converting the actual array/group documents.

extra_items= (PEP 728). msgspec ignores it. With a TypedDict(..., extra_items=int), converting {"name": "x", "k": 5} silently drops k, and it doesn't even flag {"name": "x", "k": "not_an_int"} as invalid. ArrayMetadataV3 and GroupMetadataV3 use extra_items=ExtensionFieldV3 to validate v3 extension fields, so this matters for spec-correct validation of extension keys.

these are both pretty important features! it's too bad that msgspec doesn't support them. but i don't think we need to reach 100% with just 1 tool. I would be interested in seeing how far we could get with msgspec, even if it doesn't get us all the way. The metric here should be increasing code simplicity (removing code) + catching bugs.