Skip to content

fix: tighten array metadata field validation - #4494

Open
barlowa124 wants to merge 2 commits into
zarr-developers:mainfrom
barlowa124:meta-validate-4453
Open

barlowa124 wants to merge 2 commits into
zarr-developers:mainfrom
barlowa124:meta-validate-4453

Conversation

@barlowa124

Copy link
Copy Markdown

Summary

Four array metadata fields accepted JSON outside the spec contract. shape: [true] parsed as (1,) because bool subclasses int. Document shape fields now reject booleans through parse_shapelike(..., reject_bool=True), while stored chunks/chunk_shape fields keep the lenient default since old versions wrote true there and repair tests codify reading them. attributes: [] parsed as {}. parse_attributes now requires a dict with string keys, matching the check GroupMetadata already applies. dimension_names: "xy" parsed as ("x", "y") because a bare string is iterable. Strings and bytes are excluded before the element check. storage_transformers: {"name": ...} parsed as a tuple of keys. Mappings are excluded from the iterable check.

Refs #4453, fifth checklist item.

For reviewers

The parse_shapelike signature gains a keyword-only reject_bool flag, with the lenient default preserving the repair path for stored chunk sizes. Worth a second look: attributes: null still parses to {}, matching the group's handling of missing attributes.

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)

barlowa124 and others added 2 commits October 8, 2026 16:11
Four metadata fields accepted out-of-contract JSON:

- shape accepted booleans ([true] read as (1,)); document shape fields
  now reject them via parse_shapelike(reject_bool=True). Stored
  chunks/chunk_shape fields keep the lenient default since older
  versions wrote true there and repair tests codify reading them.
- attributes accepted a list; it now requires a dict with string keys,
  matching the check GroupMetadata already applies.
- dimension_names accepted a bare string, iterating it into
  characters; strings and bytes are excluded before the element check.
- storage_transformers accepted a dict, iterating it into keys;
  mappings are excluded.

Refs zarr-developers#4453

Generated with Devin
Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@github-actions github-actions Bot added needs release notes Automatically applied to PRs which haven't added release notes and removed needs release notes Automatically applied to PRs which haven't added release notes labels Oct 8, 2026
@codecov

codecov Bot commented Oct 8, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.70%. Comparing base (069fd20) to head (bbf6b2d).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #4494   +/-   ##
=======================================
  Coverage   94.69%   94.70%           
=======================================
  Files          94       94           
  Lines       13606    13624   +18     
=======================================
+ Hits        12884    12902   +18     
  Misses        722      722           
Files with missing lines Coverage Δ
src/zarr/core/common.py 92.47% <100.00%> (+0.12%) ⬆️
src/zarr/core/metadata/common.py 100.00% <100.00%> (ø)
src/zarr/core/metadata/v2.py 90.65% <100.00%> (ø)
src/zarr/core/metadata/v3.py 96.91% <100.00%> (ø)

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

This branch has not been deployed

No deployments
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.

1 participant