Skip to content

Fix structured dtype v2 round-trip and silent loss of field offsets/padding - #188

Closed
d-v-b wants to merge 25 commits into
mainfrom
claude/structured-dtype-bugfixes
Closed

d-v-b wants to merge 25 commits into
mainfrom
claude/structured-dtype-bugfixes

Conversation

@d-v-b

@d-v-b d-v-b commented Jun 12, 2026 •

Copy link
Copy Markdown
Owner

🤖 AI text below 🤖

Nested structured dtypes now survive the Zarr V2 metadata round trip, including array write/reopen/read. Recursive native-dtype validation detects padding and alignment at every level; padded layouts warn and are repacked while preserving field values. Packed layouts and existing stored metadata remain supported. Subarray fields remain unsupported.

The tests exercise nested JSON/native round trips, padding, alignment, scalar conversion, and array I/O. The review ran 758 dtype, V2, and dtype-property tests successfully (5 existing skips). The changelog is included in changes/4340.bugfix.md.

d-v-b and others added 2 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>
d-v-b and others added 20 commits September 12, 2026 17:49
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>
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>
Correct unsupported format claims using V2 history and runtime probes. Tighten the generated dtype property so unexpected rejection fails.

Assisted-by: Codex:GPT-6
* fix: complete imagecodecs codec package hints

Assisted-by: Codex:GPT-6

* docs: associate codec changelog with PR 338

Assisted-by: Codex:GPT-6

* docs: associate codec changelog with upstream PR 4351

Assisted-by: Codex:GPT-6
…r-developers#4345)

* docs(indexing): ground design and integration claims in current behavior

Assisted-by: Codex:GPT-6

* docs(indexing): correct reader lazy-array and cache contracts

Assisted-by: Codex:GPT-6

* fix(indexing): validate wire boundaries and clarify format contracts

Assisted-by: Codex:GPT-6

* fix(indexing): validate selector bounds and shared dependencies

Correct mathematical API documentation to match supported coordinate, grid, and chunk projection contracts.

Assisted-by: Codex:GPT-6

* docs(indexing): reconcile reader contracts and record audit fixes

Assisted-by: Codex:GPT-6

* docs(indexing): reconcile audit with current partition implementation

Retain the existing unsigned selector fix and update the unsupported mixed-dependency error assertion for general intersection routing.

Assisted-by: Codex:GPT-6

* docs(indexing): clarify planning coverage and benchmark measurement boundaries

Assisted-by: Codex:GPT-6

* fix(indexing): group signed chunk coordinates without collisions

Use lexicographic tuple grouping when chunk indices contain negative values. Cover shared one-axis and two-axis array dependencies, repeated points, and extreme signed coordinates.

Assisted-by: Codex:GPT-6

* docs(indexing): state remaining planner limits precisely

Assisted-by: Codex:GPT-6

* docs(indexing): number audit changelog entries for PR 4345

Assisted-by: Codex:GPT-6

* docs(indexing): describe current contracts in docstrings

Remove implementation history and unsupported historical claims from source and test docstrings. Distinguish immutable coordinate mappings from mutable source values.

Assisted-by: Codex:GPT-6
* docs(indexing): ground design and integration claims in current behavior

Assisted-by: Codex:GPT-6

* docs(indexing): correct reader lazy-array and cache contracts

Assisted-by: Codex:GPT-6

* fix(indexing): validate wire boundaries and clarify format contracts

Assisted-by: Codex:GPT-6

* fix(indexing): validate selector bounds and shared dependencies

Correct mathematical API documentation to match supported coordinate, grid, and chunk projection contracts.

Assisted-by: Codex:GPT-6

* docs(indexing): reconcile reader contracts and record audit fixes

Assisted-by: Codex:GPT-6

* docs(indexing): reconcile audit with current partition implementation

Retain the existing unsigned selector fix and update the unsupported mixed-dependency error assertion for general intersection routing.

Assisted-by: Codex:GPT-6

* docs(indexing): clarify planning coverage and benchmark measurement boundaries

Assisted-by: Codex:GPT-6

* fix(indexing): group signed chunk coordinates without collisions

Use lexicographic tuple grouping when chunk indices contain negative values. Cover shared one-axis and two-axis array dependencies, repeated points, and extreme signed coordinates.

Assisted-by: Codex:GPT-6

* docs(indexing): state remaining planner limits precisely

Assisted-by: Codex:GPT-6

* test(indexing): broaden planner property coverage

Assisted-by: Codex:GPT-6

* test(indexing): generate mixed affine planner dependencies

Assisted-by: Codex:GPT-6

* docs(indexing): number audit changelog entries for PR 4345

Assisted-by: Codex:GPT-6

* docs(indexing): describe current contracts in docstrings

Remove implementation history and unsupported historical claims from source and test docstrings. Distinguish immutable coordinate mappings from mutable source values.

Assisted-by: Codex:GPT-6
…developers#4347)

* docs(indexing): ground design and integration claims in current behavior

Assisted-by: Codex:GPT-6

* docs(indexing): correct reader lazy-array and cache contracts

Assisted-by: Codex:GPT-6

* fix(indexing): validate wire boundaries and clarify format contracts

Assisted-by: Codex:GPT-6

* fix(indexing): validate selector bounds and shared dependencies

Correct mathematical API documentation to match supported coordinate, grid, and chunk projection contracts.

Assisted-by: Codex:GPT-6

* docs(indexing): reconcile reader contracts and record audit fixes

Assisted-by: Codex:GPT-6

* docs(indexing): reconcile audit with current partition implementation

Retain the existing unsigned selector fix and update the unsupported mixed-dependency error assertion for general intersection routing.

Assisted-by: Codex:GPT-6

* docs(indexing): clarify planning coverage and benchmark measurement boundaries

Assisted-by: Codex:GPT-6

* fix(indexing): group signed chunk coordinates without collisions

Use lexicographic tuple grouping when chunk indices contain negative values. Cover shared one-axis and two-axis array dependencies, repeated points, and extreme signed coordinates.

Assisted-by: Codex:GPT-6

* docs(indexing): state remaining planner limits precisely

Assisted-by: Codex:GPT-6

* fix(indexing): reject unsupported wire index array bounds

Assisted-by: Codex:GPT-6

* docs(indexing): clarify wire bounds rejection contract

Assisted-by: Codex:GPT-6

* docs(indexing): number audit changelog entries for PR 4345

Assisted-by: Codex:GPT-6

* docs(indexing): describe current contracts in docstrings

Remove implementation history and unsupported historical claims from source and test docstrings. Distinguish immutable coordinate mappings from mutable source values.

Assisted-by: Codex:GPT-6

* fix(indexing): validate raw index values against wire bounds

Accept valid finite and one-sided bounds with eager validation shared by both JSON loaders. Validate before affine adjustment and simplification; immutable validated maps need no retained constraint.

Assisted-by: Codex:GPT-6
…4348)

* docs(indexing): ground design and integration claims in current behavior

Assisted-by: Codex:GPT-6

* docs(indexing): correct reader lazy-array and cache contracts

Assisted-by: Codex:GPT-6

* fix(indexing): validate wire boundaries and clarify format contracts

Assisted-by: Codex:GPT-6

* fix(indexing): validate selector bounds and shared dependencies

Correct mathematical API documentation to match supported coordinate, grid, and chunk projection contracts.

Assisted-by: Codex:GPT-6

* docs(indexing): reconcile reader contracts and record audit fixes

Assisted-by: Codex:GPT-6

* docs(indexing): reconcile audit with current partition implementation

Retain the existing unsigned selector fix and update the unsupported mixed-dependency error assertion for general intersection routing.

Assisted-by: Codex:GPT-6

* docs(indexing): clarify planning coverage and benchmark measurement boundaries

Assisted-by: Codex:GPT-6

* fix(indexing): group signed chunk coordinates without collisions

Use lexicographic tuple grouping when chunk indices contain negative values. Cover shared one-axis and two-axis array dependencies, repeated points, and extreme signed coordinates.

Assisted-by: Codex:GPT-6

* docs(indexing): state remaining planner limits precisely

Assisted-by: Codex:GPT-6

* fix(indexing): define explicit source token contract

Assisted-by: Codex:GPT-6

* fix(indexing): reject mmap-backed token buffers

Assisted-by: Codex:GPT-6

* docs(indexing): number audit changelog entries for PR 4345

Assisted-by: Codex:GPT-6

* docs(indexing): describe current contracts in docstrings

Remove implementation history and unsupported historical claims from source and test docstrings. Distinguish immutable coordinate mappings from mutable source values.

Assisted-by: Codex:GPT-6

* fix(indexing): delegate source tokenization to Dask

Assisted-by: Codex:GPT-6
…arr-developers#4358)

Zarr v2 retried DirectoryStore's rename because Windows intermittently
refuses to replace a destination (zarr-developers#597, fixed by zarr-developers#698). Atomic writes
arrived in v3's LocalStore in zarr-developers#3412 without that retry, so the failure is
back: _atomic_write's tmp_path.replace(path) raises

    PermissionError: [WinError 5] Access is denied:
      '...zarr.<hex>.partial' -> '...zarr.json'

and aborts the write. Reported in zarr-developers#3522.

_move_with_retry wraps the final move, retrying only the two Windows codes
that mean the destination could not be superseded right now. It needs no
platform test: off Windows an OSError carries no winerror, so the first
attempt either succeeds or raises.

The exclusive path is routed through it too but is unaffected by
construction -- the FileExistsError it relies on to report an existing node
is ERROR_ALREADY_EXISTS (183), which is not in the retried set, so it still
propagates on the first attempt.

Measured on Windows 11, 4,000 group-attr rewrites (each a replace onto an
existing zarr.json): 155-171 raised before, 0 after, for 3.68 s -> 4.08 s of
wall clock on a workload that is nothing but replace-onto-existing. In a
narrower stdlib-only loop of 20,000 replaces, 475 of 498 recoveries needed
only the second attempt and the worst needed the fourth.
…ers#4358 (zarr-developers#4359)

Follow-up cleanup to the retry landed in zarr-developers#4358, no behaviour change:

- drop the leading 0.0 sentinel, the bare `last_error` annotation, and
  the possibly-unbound re-raise from `_move_with_retry`; the final
  attempt now runs after the loop and propagates its own traceback
- correct the comment on ERROR_ACCESS_DENIED: Windows also reports it
  for destinations that will never clear (directory, read-only, ACL),
  so those surface the same error after the bounded delay
- align the changelog with the code: the exclusive path is routed
  through the retry, only FileExistsError is excluded; rename the
  fragment to the PR number and describe zarr-developers#3522 as mitigated, since a
  second process holding the destination open past the budget still
  fails
- tests record `time.sleep` instead of sleeping (~1.2 s per run before,
  now instant) and assert the actual delay schedule; the closure test
  double becomes a small callable class, removing four type-ignores
- add a regression test for `_atomic_write` onto an existing directory

Assisted-by: ClaudeCode:claude-fable-5-1

Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
* docs(blog): skeleton for the 3.4.0 release post

Headings and front matter in the 3.3.0 house style, with the proposed
highlights, evidence, PR references and snippet ideas as markdown comments
for the prose to be written against. Marked draft: true so it is excluded
from the built site until it is ready.

Assisted-by: ClaudeCode:claude-fable-5-1

* docs(blog): 3.4.0 post prose, copy edits, references, and contributor handles

Prose for the stack, roadmap, rectilinear, codec-error and msgspec
sections; typo and grammar fixes; PR/issue links throughout; a runnable
example of the new unknown-codec error; contributors listed by GitHub
handle with first-time contributors marked. The 3.3.0 post gains the
same contributor section for its own release range.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

* docs(blog): drop sections about fixes for 3.3.0 regressions

* docs(blog): link the previous post from the contributor note

* AI disclaimer

* Update 3.4.0-release.md

* Update docs/blog/posts/3.4.0-release.md

Co-authored-by: Ilan Gold <ilanbassgold@gmail.com>

* prose

* docs: add link to registry docs

* docs: fix the 3.3.0 release-notes link that pointed at a nonexistent PR

The load/open docstring entry cited zarr-developers#3984, which does not exist upstream
as a PR or an issue, so the weekly link check (zarr-developers#4357) reports a 404.
Point at the commit that landed the change instead.

Assisted-by: ClaudeCode:claude-fable-5-1

---------

Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
Co-authored-by: Ilan Gold <ilanbassgold@gmail.com>
Built with `towncrier build --version 3.4.0`, consuming the 33 fragments
under changes/. Two touch-ups to the generated section: the codec
error-message example fence gets a `text` language (markdownlint MD040),
and the three fragments that had been named after issues rather than
the pull requests that merged them (3285 -> zarr-developers#4063; 4174 and 4272 ->
zarr-developers#4218) now link to those pull requests.

Assisted-by: ClaudeCode:claude-fable-5-1
…zarr-developers#4361)

GitHub's "Generate release notes" lists every pull request merged since
the previous tag, which in this repository includes the zarr-metadata,
zarr-indexing and zarr-http-server work that ships with those packages'
own releases. Label pull requests confined to one packages/ directory
with that package's name, and exclude those labels in .github/release.yml;
dependabot updates get their own section.

Assisted-by: ClaudeCode:claude-fable-5-1
…developers#4349)

* docs(indexing): ground design and integration claims in current behavior

Assisted-by: Codex:GPT-6

* docs(indexing): correct reader lazy-array and cache contracts

Assisted-by: Codex:GPT-6

* fix(indexing): validate wire boundaries and clarify format contracts

Assisted-by: Codex:GPT-6

* fix(indexing): validate selector bounds and shared dependencies

Correct mathematical API documentation to match supported coordinate, grid, and chunk projection contracts.

Assisted-by: Codex:GPT-6

* docs(indexing): reconcile reader contracts and record audit fixes

Assisted-by: Codex:GPT-6

* docs(indexing): reconcile audit with current partition implementation

Retain the existing unsigned selector fix and update the unsupported mixed-dependency error assertion for general intersection routing.

Assisted-by: Codex:GPT-6

* docs(indexing): clarify planning coverage and benchmark measurement boundaries

Assisted-by: Codex:GPT-6

* fix(indexing): group signed chunk coordinates without collisions

Use lexicographic tuple grouping when chunk indices contain negative values. Cover shared one-axis and two-axis array dependencies, repeated points, and extreme signed coordinates.

Assisted-by: Codex:GPT-6

* docs(indexing): state remaining planner limits precisely

Assisted-by: Codex:GPT-6

* feat(indexing): execute partitions with their reader context

Assisted-by: Codex:GPT-6

* docs(indexing): number audit changelog entries for PR 4345

Assisted-by: Codex:GPT-6

* docs(indexing): describe current contracts in docstrings

Remove implementation history and unsupported historical claims from source and test docstrings. Distinguish immutable coordinate mappings from mutable source values.

Assisted-by: Codex:GPT-6

* fix(indexing): plan every view read with its source grid

Remove Partition.result and partition-local source windows. Preserve the source grid in derived views and supply projections for every LazyArray reader call, including unpartitioned reads.

Assisted-by: Codex:GPT-6
d-v-b and others added 3 commits September 16, 2026 11:45
…ers#4350)

* feat(indexing): make LazyArray indexing lazy by default

Add synchronous writes through composed selections and an explicit eager adapter for array consumers. Cover deferred reads, source mutation, masks, aliases, and repeated destinations.

Assisted-by: Codex:GPT-6
Rebased-onto: upstream/main after zarr-developers#4345-zarr-developers#4349 merged; conflicts resolved with ClaudeCode:claude-fable-5-1

* fix(indexing): plan fancy writes against the source's write grid

The non-affine write fallback assigned one element at a time, which on a
chunked source is one chunk read-modify-write per element: a 50-row
orthogonal write into a 1000x1000 zarr array with 100x100 chunks cost
49,999 chunk writes and 27 s against 100 writes and 10 ms natively.

`write_into` now scatters in bulk. NumPy sources get one fancy assignment.
A readable source with a write grid, which `LazyArray.write` discovers
from `write_chunk_sizes` then `chunks`, is written one cell at a time:
the cell's touched hull is read once, updated in memory with
last-occurrence-wins semantics, and written back with one basic slice.
The same write into zarr now costs 100 chunk writes. Sources with no
grid, transforms the planner cannot factor, and sources that cannot be
read keep the per-element path, which never reads.

Also: order the rank check before the value copy while still validating
values for an empty selection; keep the payload of a masked zero-rank
affine write, which `np.flip` with no axes was turning into the `masked`
singleton; note that writes bypass the reader and pin that with a test;
note that iterated elements are views; add API pages for the writer and
eager adapter; and describe the shipped indexing surface in the
unreleased `LazyArray` changelog fragment instead of its removed `.lazy`
accessor.

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

* feat(indexing): keep literal domains on views, positional keys unchanged

`LazyArray` re-zeroed the domain of every derived view, so a view forgot
which coordinates it was cut from and placement needed a side channel.
TensorStore keeps literal coordinates on views; NumPy users expect
positional keys. These are separable: the key dialect says how a key is
read, the domain says what the view remembers.

Views now keep their literal domain: `a[10:20]` has domain `[10, 20)`
and `a[10:20][2:5]` has `[12, 15)`, while `a[10:20][0]` is still the
first element because positional keys are normalized against the
domain's origin, which `normalize_positional_selection` already did.
Two literal keys join the NumPy ones, as in TensorStore's `__getitem__`:
an `IndexDomain` restricts the view to coordinates of its own domain
(empty intervals outside it are refused too) and an `IndexTransform`
composes onto it. Box partitions keep the request's coordinates, so a
part view's domain is a sub-domain of its parent's; a part placed by
index arrays keeps a fresh zero-origin domain, and `out_selection` is
the placement in both cases. A reversed view shows the negative origin
the algebra already produced.

Two frames stay zero-origin by construction so no consumer has to
subtract an origin: `ReadContext` re-bases its transform, and `parts()`
re-bases each projection's `cell_transform` from the request's literal
domain to positions in the view's result buffer, which is what
`Partition.projection` has always documented.

A table-driven test states the expected literal domain for every
indexing form and nested chain, checked against the transform algebra.

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

* docs(indexing): state that a key's type picks relative or absolute indexing

Make the one rule about indexing a LazyArray prominent: NumPy keys are
positions relative to the view, an `IndexDomain` key names absolute
coordinates of the view's domain, an `IndexTransform` key composes onto
it, no key type has two readings, and every view keeps its absolute
domain whichever key produced it. A guide section with a runnable
snippet carries the table and the pandas `ix` / TensorStore comparison;
the module docstring, README, and API page point at it.

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

---------

Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
…elopers#4363)

* fix: replace runtime assert statements with explicit checks

Asserts are stripped under `python -O`, so checks that matter for
correctness or type narrowing must be explicit. Two were load-bearing:
`GroupMetadata.from_dict` asserted on node_type and the array-to-group
fallback in `zarr.api.asynchronous.open` caught the AssertionError, and
`make_store` asserted on mode ahead of the real validation.

Redundant asserts are deleted, narrowing asserts are restructured so
mypy narrows on its own, and the rest become explicit raises. Ruff S101
is enabled with `tests/` and `src/zarr/testing/` excluded.

Assisted-by: ClaudeCode:claude-fable-5-1

* docs: add changelog fragment for zarr-developers#4363

Assisted-by: ClaudeCode:claude-fable-5-1

* ci: scope the no-assert lint to runtime code across the monorepo

The S101 per-file ignores only covered the root tests/ and
src/zarr/testing/. The packages under packages/ inherit the root ruff
config, so the rule fired on 1524 asserts in their tests, examples, and
test-support modules and broke the ruff, Lint, pre-commit.ci, and
zarr-http-server jobs. Widen the ignores to **/tests/**, **/examples/**,
and **/testing/**, and exclude zarr-indexing's runtime source for now;
its own asserts are tracked as a separate change.

Assisted-by: ClaudeCode:claude-fable-5-1
@d-v-b

d-v-b commented Sep 24, 2026

Copy link
Copy Markdown
Owner Author

🤖 AI text below 🤖

Closing: merged upstream as zarr-developers#4340.

@d-v-b d-v-b closed this Sep 24, 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.

2 participants