fix(dtype): preserve scaled generic time units and normalize the μs alias - #4342
Draft
d-v-b wants to merge 9 commits into
Draft
fix(dtype): preserve scaled generic time units and normalize the μs alias#4342d-v-b wants to merge 9 commits into
d-v-b wants to merge 9 commits into
Conversation
…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>
DateTime64 and TimeDelta64 accepted two values that did not survive a round trip through NumPy: unit="generic" with scale_factor != 1 (NumPy's generic time type carries no scale, so np.dtype silently dropped it and the Zarr V2 dtype string lost it while the V3 metadata kept it), and unit="μs" (NumPy only reports "us", so the instance compared unequal to itself after to_native_dtype/from_native_dtype). The constructor now raises for the first and stores "us" for the second; metadata spelling the unit "μs" still reads. Assisted-by: ClaudeCode:claude-fable-5-1 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Add numpy_datetime64_configuration, numpy_timedelta64_configuration and the shared numpy_time_unit validator, following the leaf-validator precedent (hex_float16, raw_bytes_dtype_name) for constraints the configuration TypedDicts cannot express: scale_factor must be an integer in [1, 2**31 - 1], the generic unit takes scale_factor 1 only, and the equivalent "μs" spelling of the microsecond unit is normalized to "us". The unit vocabulary, previously duplicated in both leaf modules, moves to a private shared module and is re-exported unchanged. Assisted-by: ClaudeCode:claude-fable-5-1 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The DateTime64/TimeDelta64 constructor now normalizes "μs" and rejects a scaled generic unit, so the strategy draws every unit and conditions the scale factor on the unit instead of filtering and post-hoc rewriting. 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>
Documentation build overview
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4342 +/- ##
==========================================
+ Coverage 94.34% 94.36% +0.02%
==========================================
Files 92 92
Lines 12935 13041 +106
==========================================
+ Hits 12203 12306 +103
- Misses 732 735 +3
🚀 New features to boost your workflow:
|
Assisted-by: Codex:GPT-6
Assisted-by: Codex:GPT-6
d-v-b
changed the title
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
1 participant
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.
🤖 AI text below 🤖
Preserves temporal dtype parameters through NumPy conversion and Zarr metadata, on top of the structured dtype changes in #4340.
DateTime64andTimeDelta64normalize the equivalent microsecond spellingsμsandustous.Generic units accept the documented integer scale range. NumPy retains a scale in
np.dtype("M8[2generic]"), butdtype.nameanddtype.stromit it. Native conversion now reads the dtype object directly. V2 serialization emits an explicit suffix such as<M8[2generic]; V3 retains the unit and scale in its configuration. NumPy and the Zarr2.18.7 dtype decoder accept that explicit V2 string. The previous blanket rejection of generic scales has been removed from both the core dtype classes and the optional metadata validators.Array-level regressions exposed further problems: generic datetime integer fill values failed to decode; NumPy CPU allocation could discard generic scale or byte order; and NumPy generic-time
astypecould change the endian marker without swapping counts. The fix preserves allocation dtypes, converts endian-only casts through integer counts, and supports zero-valued generic datetime fields in structured defaults. These workarounds are scoped to the affected temporal paths. No GPU validation is claimed.Tests exercise both temporal types, all supported units, scale boundaries, byte orders, native/V2/V3 conversions, plain and structured array IO, and missing chunks. The dtype Hypothesis strategy now generates scaled generic units instead of excluding them. Invalid-unit/type/range cases remain separate. The metadata model's pydantic path remains structural; leaf validators apply semantic checks when explicitly called.
Validation: 1,074 dtype/buffer tests passed (18 expected skips), 57 metadata temporal tests passed, and all three dtype property tests passed. New regressions failed before the corresponding fixes. Commit hooks also passed. The legacy decoder check verifies the dtype string, not universal compatibility with every historical array operation.