Fix NameError in v3 from_zarr/ArraySpec.like and KeyError in model_dump#169
Merged
Conversation
- ArraySpec.like in v3.py used bare `isinstance(other, zarr.Array)` with zarr
only imported under TYPE_CHECKING, causing NameError at runtime; switch to
the sys.modules guard already used by GroupSpec.like.
- Module-level from_zarr in v3.py and experimental/v3.py referenced zarr.Array
with no runtime import; add `import zarr` inside the function body (mirrors v2.py).
- ArraySpec.model_dump in both v3.py and experimental/v3.py did
d["dimension_names"] unconditionally, raising KeyError when dimension_names
was excluded via model_dump(exclude={...}); use d.get() + pop(key, None).
- Fix conftest.py for zarr 3.2 compatibility: Structured was renamed to Struct
and requires fields; detect whichever name is present in the registry.
- Bump mypy python_version from 3.11 to 3.12 so mypy can parse zarr 3.2
which uses the PEP 695 `type` statement syntax.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…antic-zarr into fix/v3-zarr-nameerror
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.
Summary
This PR fixes four verified runtime crashes in
pydantic_zarr.v3andpydantic_zarr.experimental.v3, identified in #165.Bug 1:
ArraySpec.likeraisesNameErrorwhen called with a spec or zarr arrayArraySpec.likeinsrc/pydantic_zarr/v3.pydid a bareisinstance(other, zarr.Array)butzarris only imported underTYPE_CHECKING, so at runtime this crashes:Fix: Use the
sys.modulesguard thatGroupSpec.likealready uses:Bug 2: Module-level
from_zarrinv3.pyraisesNameErrorThe function body does
isinstance(element, zarr.Array)with no runtime import:Fix: Add
import zarrinside the function body, matchingv2.py.Bug 3: Module-level
from_zarrinexperimental/v3.pyraisesNameErrorSame as Bug 2 but in the experimental module.
Fix: Add
import zarrinside the function body.Bug 4:
ArraySpec.model_dump(exclude={'dimension_names'})raisesKeyErrorThe
model_dumpoverride in bothv3.pyandexperimental/v3.pydidd["dimension_names"]unconditionally. When the field is excluded, the key is absent:Fix: Use
d.get("dimension_names") is Noneandd.pop("dimension_names", None).Additional fixes
conftest.pyzarr 3.2 compatibility: zarr 3.2 renamedStructuredtoStruct(with different constructor signature); the test conftest crashed on import with zarr 3.2. Updated to detect whichever name is present in the dtype registry.python_version: Bumped from"3.11"to"3.12"so mypy can parse zarr 3.2 code that uses the PEP 695typestatement.Test plan
test_arrayspec_like_spec_vs_spec—ArraySpec.like(spec)no longer raisesNameErrortest_arrayspec_like_spec_vs_zarr_array—ArraySpec.like(zarr_array)no longer raisesNameErrortest_from_zarr_array/test_from_zarr_group— module-levelfrom_zarrno longer raisesNameErrortest_model_dump_exclude_dimension_names—model_dump(exclude={'dimension_names'})no longer raisesKeyErrorexperimental.v3🤖 Generated with Claude Code