Skip to content

fix(dtype): fix nested v2 round trips and detect unsupported field conversions - #4340

Open
d-v-b wants to merge 9 commits into
zarr-developers:mainfrom
d-v-b:claude/structured-dtype-bugfixes
Open

fix(dtype): fix nested v2 round trips and detect unsupported field conversions#4340
d-v-b wants to merge 9 commits into
zarr-developers:mainfrom
d-v-b:claude/structured-dtype-bugfixes

Conversation

@d-v-b

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

Copy link
Copy Markdown
Contributor

🤖 AI text below 🤖

Fix nested structured dtype round-tripping in V2 metadata, detect field features unsupported by the current native dtype conversion, and warn when that conversion changes layout.

  • Validate nested V2 field definitions as lists of fields. The baseline can emit nested metadata that its own dtype parser rejects.
  • Reject native NumPy field titles and subarray fields, including nested ones, with a ValueError identifying the field. On the baseline, dtype conversion treats string title aliases as additional fields and subarray fields as raw bytes. Array writes using these inputs can consequently fail with unrelated errors.
  • Preserve the baseline's conversion of padded layouts to packed layouts, adding a warning that field values are preserved when writing arrays but offsets and itemsize may change. The integration tests cover aligned layouts, nested padding and explicit offsets in both formats.

Format and implementation boundaries

Subarray rejection is a limitation of the current Zarr-Python dtype implementation, not of V2. The V2 specification explicitly supports [fieldname, datatype, shape]. Nested fields and subarrays were clarified in 2018's #296. A probe using Zarr-Python 2.18.7 successfully writes and reopens a structured subarray field. Restoring that support is outside this change.

For titles, NumPy's descr supports (title, name) pairs. Zarr adopted dtype.descr in April 2016, but the JSON decoder did not reconstruct those pairs as tuples. The clarified V2 grammar describes field names as strings. The reviewed history provides no explicit rationale for excluding titles; this PR makes no claim that they are inherently unrepresentable.

The V3 struct extension separately specifies named, fixed-size fields and packed encoding. Its constraints should not be applied retroactively to V2.

Validation

  • 770 dtype, V2 and property tests passed; 5 existing skips.
  • New strategies sample built-in dtype classes, bounded parameters and nested layouts. They do not claim exhaustive coverage or support arbitrary third-party dtype constructors. Datetime generation excludes the microsecond alias and normalizes generic-unit scale factors; those normalization issues remain separate.
  • The structured-dtype property independently classifies NumPy field descriptors. It requires rejection for the currently unsupported features and successful conversion for the other generated cases, checking packed dtype equality and layout warnings. An injected unrelated ValueError is detected rather than counted as success.
  • Compared runtime behavior of Zarr-Python 2.18.7, the PR baseline ba883a556, and this branch. The statements above distinguish those versions deliberately.

Supersedes d-v-b#188.

d-v-b and others added 3 commits September 12, 2026 17:35
…adding

Bug 1: Nested structured dtypes failed the Zarr V2 JSON round-trip. The inner
type guard check_structured_dtype_v2_inner recursed into itself for a nested
field's last element, but that element is a list of [name, dtype] field pairs,
not a single pair. It now validates that element with check_structured_dtype_name_v2,
so metadata written by to_json(zarr_format=2) can be read back.

Bug 2: Structured dtypes with non-default (aligned / padded) field layouts were
silently re-packed contiguously on round-trip, changing field offsets and itemsize
and corrupting stored bytes. from_native_dtype now detects non-packed layouts
(recursively) and raises a clear ValueError instead. Reading existing packed data
is unaffected.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…2 round trip

Nested structured dtypes written with Zarr format 2 could not be read back,
because the inner data type name check recursed as if the nested field were a
single [name, dtype] pair instead of a list of fields.

Structured dtypes that NumPy allows but the Zarr struct metadata cannot record
were accepted and rebuilt as something else: aligned or explicitly offset
layouts came back packed with a different itemsize, a titled field came back
as two fields (the fields mapping lists titles as extra keys), and a subarray
field came back as raw bytes. Struct.from_native_dtype now raises ValueError
naming the offending field for all three, and iterates over names rather than
the fields mapping.

Add zdtypes() and structured_dtypes() hypothesis strategies to
zarr.testing.strategies, and three property tests: every registered data type
round-trips through JSON and through NumPy, and structured dtype resolution
either raises or returns the input dtype exactly. The last property is the one
every bug above violated.

Assisted-by: ClaudeCode:claude-fable-5-1
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Assisted-by: ClaudeCode:claude-fable-5-1
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Assisted-by: ClaudeCode:claude-fable-5-1
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@d-v-b
d-v-b marked this pull request as ready for review September 12, 2026 15:57
@d-v-b

d-v-b commented Sep 12, 2026

Copy link
Copy Markdown
Contributor Author

🎵 self-merging when tests pass 🎵

d-v-b and others added 2 commits September 12, 2026 18:04
Keep packed conversion for non-default layouts with a user warning, and verify field values survive both Zarr formats.

Assisted-by: Codex:GPT-6
…ection message

Repack the dtype once at the top level before resolving fields, so a padded
nested field warns once rather than once per level of nesting. Attribute the
warning to the first frame outside the zarr package; this reaches the caller
for direct uses of the dtype API, while the synchronous array API runs on the
event loop thread where no caller frame exists.

The title/subarray error message no longer claims that stored bytes would be
misinterpreted; the field information simply has no place in the metadata.
The changelog now says those dtypes used to fail with an unrelated NumPy error
rather than losing information silently.

Assisted-by: ClaudeCode:claude-fable-5-1
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@codecov

codecov Bot commented Sep 12, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.39%. Comparing base (ba883a5) to head (209a235).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4340      +/-   ##
==========================================
+ Coverage   94.34%   94.39%   +0.05%     
==========================================
  Files          92       92              
  Lines       12935    13016      +81     
==========================================
+ Hits        12203    12286      +83     
+ Misses        732      730       -2     
Files with missing lines Coverage Δ
src/zarr/core/dtype/common.py 88.50% <100.00%> (+2.29%) ⬆️
src/zarr/core/dtype/npy/structured.py 93.64% <100.00%> (+1.02%) ⬆️
src/zarr/testing/strategies.py 96.66% <100.00%> (+0.52%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Correct unsupported format claims using V2 history and runtime probes. Tighten the generated dtype property so unexpected rejection fails.

Assisted-by: Codex:GPT-6
@d-v-b d-v-b changed the title fix(dtype): reject unrepresentable structured dtypes and fix nested v2 round trip fix(dtype): fix nested v2 round trips and detect unsupported field conversions Sep 13, 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.

1 participant