Repository navigation
feat(zarr-metadata)!: write store bytes through a JSONWriter - #4610
Merged
d-v-b merged 3 commits intoNov 9, 2026
Merged
Conversation
`to_key_value` takes a writer instead of an `indent` keyword. A
`JSONWriter` owns how a document becomes JSON bytes: it holds the
settings (`indent`, `sort_keys`, `ensure_ascii`, `trailing_newline`)
and its `encode` method does the writing, so the separators, the bare
non-finite tokens and the surrogate escapes are no longer split off in
a private helper. `to_key_value` also accepts a mapping of the settings,
`JSONWriterSchema`, so callers need not import the class:
`to_key_value({"indent": 2})`.
The defaults are those of `json.dumps`, so a bare `to_key_value()` writes
the same bytes as before. A setting `json.dumps` would misread raises,
and so does a mapping key that is not a setting.
Rebased onto main as one commit, replacing the JSONWriterConfig design.
Assisted-by: ClaudeCode:claude-opus-5-5
…s#4610 Assisted-by: ClaudeCode:claude-opus-5-5
Documentation build overview
|
d-v-b
marked this pull request as ready for review
November 9, 2026 13:34
This was referenced Nov 9, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This factors out the details of configuring the JSON output of a metadata document to a dedicated
JSONWriterclass, so that we don't end up filling up**kwargsof methods that write JSON with implementation details ofjson.dumps. Users can declare aJSONWriterexplicitly, or via a configuration dict.cc @zarr-developers/python-core-devs, I think this pattern (a single entity, configured and passed to the relevant routines, controls JSON writing) is nicer than what we are currently doing in
zarritself, where we control JSON serialization indirectly with the global config object.This is a zarr-metadata-specific so ima merge it when green.
🤖 AI text below 🤖
Agent summary
Breaking:
to_key_valuetakes a writer instead of anindentkeyword. AJSONWriterowns how a document becomes JSON bytes: it holds the settings (indent,sort_keys,ensure_ascii,trailing_newline), and itsencodemethod does the writing.to_key_valuealso accepts a plain mapping of the settings, soto_key_value(indent=2)becomesto_key_value({"indent": 2}), with no import.JSONWriteris a final, frozen dataclass inzarr_metadata.model._json_writer, exported fromzarr_metadataandzarr_metadata.model.encodeholds every choice about the bytes: the separators of Python'sjsonmodule, non-finite numbers written as bare tokens (as zarr-python writes attributes), and lone surrogates written as escapes whenensure_asciiis off. The privatedump_store_jsonhelper is gone, so nothing else writes store JSON.JSONWriterSchemais the closedTypedDictof the settings, every one optional.JSONWriterLike = JSONWriter | JSONWriterSchemais what each model'sto_key_value(writer=...)accepts.JSONWriter.from_dictturns either into a writer (a writer passes through), andto_dictgoes back.json.dumps, which the models already used, so a bareto_key_value()writes the same bytes as before.json.dumpswould accept but misread raises when the writer is built:indent=True(read as 1), a negative indent (read as 0), or a string indent that is not JSON whitespace (written verbatim, giving invalid JSON). A mapping key that is not a setting raisesTypeError, so a misspelled setting is not ignored.docs/models.mdgains a "Writing store bytes" section with a tested example.tests/model/test_json_writer.pychecks, for all five model types and a range of writers given both asJSONWriterand as mappings, that each key's bytes are whatjson.dumpswrites under the same settings, and that the store reads back to an equal model. Each refused value has its own test.The design moved from an earlier
JSONWriterConfigsettings object, which a separate private function read, to one object that both holds the settings and encodes, and accepts a mapping so callers need not import it. The agent rebased ontomainas one commit and had a separate agent review the redesign. That review found no correctness bugs, and its two actionable points (subclasses passing throughfrom_dict, and gaps in the success cases) are fixed here.🤖 Generated with Claude Code