Skip to content

chore(metadata): import Literal at module level in v2.py - #4327

Open
d-v-b wants to merge 2 commits into
zarr-developers:mainfrom
d-v-b:chore/json-parse-cleanups
Open

chore(metadata): import Literal at module level in v2.py#4327
d-v-b wants to merge 2 commits into
zarr-developers:mainfrom
d-v-b:chore/json-parse-cleanups

Conversation

@d-v-b

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

Copy link
Copy Markdown
Contributor

AI-written PR that makes imports happen more correctly. Self-merging when green.

🤖 AI text below 🤖

Summary

A leftover from #4063: src/zarr/core/metadata/v2.py imported Literal under TYPE_CHECKING and then again inside parse_zarr_format at call time, because the function uses it at runtime for the parse_field type argument. Move it to the module-level typing import like every other module touched by #4063.

This PR originally also corrected the json_parse module docstring and the off-by-one boundary of the attribute nesting-depth limit. #4330 removes that limit (and the validate_json_value helper) altogether, which makes those changes moot, so they have been dropped here; only the import move remains. No changelog fragment is needed for an import move.

Found during the pre-release review for #4256.

For reviewers

A three-hunk import move with no behavior change. If #4330 lands first this still applies cleanly; the two PRs no longer touch the same lines.

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 (n/a)
  • Add docstrings and API docs for any new/modified user-facing classes and functions (n/a)
  • New/modified features documented in docs/user-guide/*.md (n/a)
  • Changes documented as a new file in changes/ (n/a — import move)
  • GitHub Actions have all passed
  • Test coverage is 100% (Codecov passes)

@codecov

codecov Bot commented Sep 9, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.30%. Comparing base (39fb609) to head (85d1d76).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4327      +/-   ##
==========================================
- Coverage   94.30%   94.30%   -0.01%     
==========================================
  Files          92       92              
  Lines       12912    12911       -1     
==========================================
- Hits        12177    12176       -1     
  Misses        735      735              
Files with missing lines Coverage Δ
src/zarr/core/metadata/v2.py 89.38% <100.00%> (-0.06%) ⬇️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

parse_zarr_format needs Literal at runtime for its parse_field type
argument, so zarr-developers#4063 added a function-local import next to the existing
TYPE_CHECKING one. Import it once at module level like the other modules
that commit touched.

The docstring and depth-limit corrections this branch originally carried
are superseded by zarr-developers#4330, which removes the attribute depth limit and the
validate_json_value helper entirely.

Assisted-by: ClaudeCode:claude-fable-5-1
@d-v-b
d-v-b force-pushed the chore/json-parse-cleanups branch from b59d7a7 to 1636969 Compare September 9, 2026 13:21
@d-v-b d-v-b changed the title fix(json_parse): correct docstrings and depth-limit boundary chore(metadata): import Literal at module level in v2.py Sep 9, 2026
@github-actions github-actions Bot added the needs release notes Automatically applied to PRs which haven't added release notes label Sep 9, 2026
@d-v-b
d-v-b marked this pull request as ready for review September 9, 2026 13:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs release notes Automatically applied to PRs which haven't added release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant