Skip to content

fix: accept must_understand=false extra fields in v3 group metadata - #4391

Merged
d-v-b merged 6 commits into
zarr-developers:mainfrom
anishmehta24:fix/group-metadata-extra-fields
Sep 30, 2026
Merged

d-v-b merged 6 commits into
zarr-developers:mainfrom
anishmehta24:fix/group-metadata-extra-fields

Conversation

@anishmehta24

Copy link
Copy Markdown

Summary

Opening a v3 group whose zarr.json has an extra field like "my_ext": {"must_understand": false} fails with a TypeError, even though the spec says those fields must be accepted. #3530 fixed this for arrays but not groups.

Group metadata now keeps these fields in extra_fields and writes them back out, same as arrays do. Any other unknown key raises MetadataValidationError.

Closes #3523

For reviewers

extra_fields is a new field on GroupMetadata. I kept it the same shape as the array one.

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)

@codecov

codecov Bot commented Sep 24, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.46%. Comparing base (c79f736) to head (7f355ee).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #4391   +/-   ##
=======================================
  Coverage   94.45%   94.46%           
=======================================
  Files          93       93           
  Lines       13224    13234   +10     
=======================================
+ Hits        12491    12501   +10     
  Misses        733      733           
Files with missing lines Coverage Δ
src/zarr/core/group.py 95.60% <100.00%> (+0.04%) ⬆️
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.

@d-v-b

d-v-b commented Sep 29, 2026

Copy link
Copy Markdown
Contributor

🤖 AI text below 🤖

The changelog fragment changes/3523.bugfix.md should be renamed to changes/4391.bugfix.md. changes/README.md asks for fragments named after the pull request number, and towncrier renders the number as a link to /pull/<number> (issue_format in pyproject.toml). As named, the release notes would link to /pull/3523; #3523 is the issue this fixes.

git mv changes/3523.bugfix.md changes/4391.bugfix.md

This PR is planned for the 3.4.1 patch release.

@d-v-b d-v-b added this to the 3.4.1 milestone Sep 29, 2026
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
@read-the-docs-community

Copy link
Copy Markdown

@read-the-docs-community

Copy link
Copy Markdown

Documentation build overview

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

  🔍 Preview build  

1 file changed
± release-notes/index.html

…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
`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
@d-v-b

d-v-b commented Sep 29, 2026

Copy link
Copy Markdown
Contributor

I pushed a few refinements, thanks for this @anishmehta24

@d-v-b
d-v-b merged commit 9677081 into zarr-developers:main Sep 30, 2026
40 checks passed
d-v-b added a commit to d-v-b/zarr-python that referenced this pull request Sep 30, 2026
Resolves the import conflict in group.py with zarr-developers#4391: keep both import lines.

Assisted-by: ClaudeCode:claude-fable-5-1
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
d-v-b added a commit to d-v-b/zarr-python that referenced this pull request Sep 30, 2026
Resolves the import conflict in group.py with zarr-developers#4391: keep both import lines.

Assisted-by: ClaudeCode:claude-fable-5-1
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
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