Skip to content

fix(group): accept must_understand=false extra fields in v3 group metadata - #4451

Closed
d-v-b wants to merge 6 commits into
zarr-developers:mainfrom
d-v-b:fix/group-metadata-extra-fields
Closed

d-v-b wants to merge 6 commits into
zarr-developers:mainfrom
d-v-b:fix/group-metadata-extra-fields

Conversation

@d-v-b

@d-v-b d-v-b commented Sep 29, 2026

Copy link
Copy Markdown
Contributor

🤖 AI text below 🤖

Zarr format 3 group metadata now accepts extra fields that are objects with "must_understand": false, as the specification requires, and keeps them when the metadata is written back. Other extra fields raise MetadataValidationError instead of a TypeError. GroupMetadata takes these fields as extra_fields, which rejects keys reserved by the group metadata document and is not supported for Zarr format 2 groups. The GroupMetadata and ArrayV3Metadata constructors now reject extra fields that are not objects with "must_understand": false, like reading a metadata document does.

Closes #3523

For reviewers

  • Arrays and groups now share one validation site for extra fields: parse_extra_fields in zarr/core/metadata/v3.py checks reserved-key collisions (ValueError) and must_understand values (MetadataValidationError). extract_extra_fields splits a document's extra keys off for both from_dicts and delegates to it.
  • Zarr format 2 groups refuse extra fields at construction. Reading a v2 .zgroup still drops unknown keys, as before (including a key named extra_fields, which previously would have reached the constructor).
  • Behavior change for arrays: ArrayV3Metadata(extra_fields=...) used to accept values its own from_dict rejects; it now raises. The array's from_dict error message also changed wording (and gained the missing spaces between sentences).

Author attestation

  • I am a human, these are my changes, and I have reviewed and understood every change and can explain why each is correct.

TODO

  • Add unit tests and/or doctests in docstrings
  • Add docstrings and API docs for any new/modified user-facing classes and functions
  • New/modified features documented in docs/user-guide/*.md
  • Changes documented as a new file in changes/
  • GitHub Actions have all passed
  • Test coverage is 100% (Codecov passes)
Relationship to #4391

This PR builds on #4391 by @anishmehta24 and keeps their commit (e6b6130) as the first commit. The two later commits (array parity, v2 refusal, constructor must_understand validation) were also pushed to #4391's branch; this PR carries the same content from a maintainer fork. Only one of the two should merge.

🤖 Generated with Claude Code

anishmehta24 and others added 4 commits September 24, 2026 07:22
Share the extra-field checks between array and group metadata:
`parse_extra_fields` takes the reserved keys, and the new
`extract_extra_fields` splits and validates extra keys in `from_dict`.
`GroupMetadata` now rejects extra fields that collide with its own keys,
refuses extra fields on Zarr format 2 groups, and no longer passes a v2
`.zgroup` key named `extra_fields` to its constructor. Rename the
changelog fragment to the PR number.

Assisted-by: ClaudeCode:claude-opus-5-5
…tructors

`parse_extra_fields` now also checks that every extra field is an object
with `"must_understand": false`, so `ArrayV3Metadata` and `GroupMetadata`
can no longer be constructed with extra fields that their own `from_dict`
would reject. `extract_extra_fields` delegates to it, leaving one place
that validates extra fields. The Zarr format 2 group refusal is checked
first so it keeps its own message.

Assisted-by: ClaudeCode:claude-opus-5-5
@codecov

codecov Bot commented Sep 29, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.43%. Comparing base (319bfa4) to head (b5f40c8).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #4451   +/-   ##
=======================================
  Coverage   94.43%   94.43%           
=======================================
  Files          93       93           
  Lines       13219    13229   +10     
=======================================
+ Hits        12483    12493   +10     
  Misses        736      736           
Files with missing lines Coverage Δ
src/zarr/core/group.py 95.26% <100.00%> (+0.05%) ⬆️
src/zarr/core/metadata/v3.py 95.44% <100.00%> (ø)
src/zarr/core/sync_group.py 100.00% <ø> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

`extract_extra_fields` filtered the reserved keys and then called
`parse_extra_fields`, whose collision check could never fire from that
path. Both `from_dict`s now do the filter inline and call
`parse_extra_fields` directly, so one function owns the extra-field
contract. Also tighten the changelog fragment, the
`GROUP_METADATA_KEYS` docstring, and the stale repr comments.

Assisted-by: ClaudeCode:claude-fable-5-1
@read-the-docs-community

Copy link
Copy Markdown

@read-the-docs-community

Copy link
Copy Markdown

Documentation build overview

📚 zarr-indexing | 🛠️ Build #34838306 | 📁 Comparing adbe9e8 against latest (319bfa4)

  🔍 Preview build  

1 file changed
± release-notes/index.html

@d-v-b d-v-b closed this Sep 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

metadata parsing rejects zarr metadata with unknown fields

2 participants