Skip to content

Allow zarr 3.3; strictly exclude root node from consolidated metadata - #31

Merged
eagmon merged 2 commits into
mainfrom
chore/zarr-3.3
Aug 21, 2026
Merged

Allow zarr 3.3; strictly exclude root node from consolidated metadata#31
eagmon merged 2 commits into
mainfrom
chore/zarr-3.3

Conversation

@eagmon

@eagmon eagmon commented Aug 21, 2026

Copy link
Copy Markdown
Member

vEcoli PR #414 moved onto zarr 3.3. This makes viva-emitters compatible so downstreams (v2ecoli, the map-reduce engine #32) can adopt it.

Changes

  1. Widen the [xarray]/[dev] pin zarr~=3.1.6zarr>=3.1.6,<3.4. The full suite passes unchanged on both zarr 3.1.6 (the locked default) and zarr 3.3.0 (fresh venv). Universal resolution keeps the lock on 3.1.6 (Python-3.11-compatible); the widened range lets a Python-3.12 downstream select 3.3. Bounded at <3.4 because the emitter uses zarr-internal symbols.
  2. Strictly exclude the root node from consolidated metadata (consolidate_metadata / reconsolidate_metadata). Per @ntfrgl's review: a "" root entry is a self-reference that risks infinite loops in Zarr traversal functions beyond _flat_to_nested() (e.g. Group.tree()). viva already carries a custom group_tree_async() with an explicit if key == "": continue self-loop guard precisely to survive that root entry — which confirms the hazard. The root's own metadata still lives on the group itself, so readers keep seeing root attrs (provenance test passes). Mirrors vEcoli be603b5d.

Correction to my earlier note: I first concluded no consolidation change was needed because zarr 3.3.0's _flat_to_nested() still tolerates the root key. That basis was too narrow — the exclusion is about latent infinite-loop risk in other traversal functions, not just _flat_to_nested. Fixed.

Python support

zarr 3.3 dropped Python 3.11, but the [xarray] backend is already 3.12-only at runtime (ImportError guard). requires-python stays >=3.11 for the SQLite/parquet emitters.

Follow-up

With the root excluded, the self-loop guard in the (currently unused) group_tree_async() is now moot and could be removed in a separate cleanup.

Verification

Full suite passes on zarr 3.1.6 (107) and zarr 3.3.0 (107), incl. consolidate/reconsolidate/append/provenance/colony.

Part of the PR #414 reconciliation (follows #29, #30; precedes #32).

🤖 Generated with Claude Code

vEcoli PR #414 moved onto zarr 3.3; this makes viva-emitters compatible so
downstreams (v2ecoli, the forthcoming map-reduce engine) can adopt it.

Widen the [xarray]/[dev] pin `zarr~=3.1.6` -> `zarr>=3.1.6,<3.4`. Verified the
existing emitter code passes the full suite unchanged on BOTH zarr 3.1.6 (the
locked default) and zarr 3.3.0 (fresh venv) — no code change is needed.

Notably, the root-node consolidation "fix" upstream applied for 3.3
(be603b5d, dropping the `{"": ...}` entries) is NOT required here: zarr
3.3.0's ConsolidatedMetadata._flat_to_nested() still tolerates the root key,
and the consolidate/reconsolidate/append tests pass with viva's existing
zarr_writer. Left as-is to avoid changing consolidation behavior.

Python support: zarr 3.3 dropped Python 3.11, but the [xarray] backend is
already 3.12-only at runtime (ImportError guard in
viva_emitters/xarray_emitter/__init__.py). requires-python stays >=3.11 for
the SQLite/parquet emitters; universal resolution keeps the lock on 3.1.6
(3.11-compatible) while the widened range lets a 3.12 downstream select 3.3.
Bounded at <3.4 because the emitter relies on zarr-internal symbols.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@ntfrgl

ntfrgl commented Aug 21, 2026

Copy link
Copy Markdown

Root node metadata should be strictly excluded from Zarr's consolidated metadata, in order to avoid potential infinite loops in Zarr library functions besides ConsolidatedMetadata._flat_to_nested(). For example, this was the purpose of the custom version of Group.tree() with an explicit guard, which could be removed in CovertLab/vEcoli@be603b5.

Per @ntfrgl's review on PR #31: a "" (root) entry in consolidated metadata is
a self-reference that risks infinite loops in Zarr traversal functions beyond
ConsolidatedMetadata._flat_to_nested() — e.g. Group.tree(). viva already
carries a custom group_tree_async() reimplementation with an explicit
`if key == "": continue` self-loop guard precisely to survive that root entry,
which confirms the hazard.

Stop inserting the root "" into consolidated metadata in both
consolidate_metadata() (drop `members_metadata |= {"": group.metadata}`) and
reconsolidate_metadata() (guard the root out of the mod/add members with
`if k`, and drop `| {""}` from old_keys). The root's own metadata continues to
live on the group itself, so readers still see root attrs (provenance test
passes). Mirrors vEcoli be603b5d.

The now-moot self-loop guard in the (currently unused) group_tree_async() is
left for a separate cleanup.

Verified: full suite passes on zarr 3.1.6 (107) and zarr 3.3.0 (107), incl.
consolidate/reconsolidate/append/provenance/colony.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@eagmon eagmon changed the title Allow zarr 3.3 (widen xarray-extra pin) Allow zarr 3.3; strictly exclude root node from consolidated metadata Aug 21, 2026
@eagmon

eagmon commented Aug 21, 2026

Copy link
Copy Markdown
Member Author

Fixed in 63fd78c — the root node is now strictly excluded from consolidated metadata in both consolidate_metadata() and reconsolidate_metadata().

You're right that my original basis was too narrow: I'd checked only that zarr 3.3.0's ConsolidatedMetadata._flat_to_nested() still tolerates the "" key, and missed the broader traversal-function hazard. viva's own custom group_tree_async() with the if key == "": continue self-loop guard is exactly the evidence — it exists to survive the root entry. With the root now excluded, that guard is moot; I've left removing it (and the unused custom group_tree, in favor of Group.tree()) as a follow-up cleanup rather than expand this PR.

Verified the full suite on both zarr 3.1.6 and 3.3.0 (107 each), including consolidate/reconsolidate/append/provenance/colony; root attrs (provenance) still read back fine since the root's metadata lives on the group itself.

@eagmon
eagmon merged commit e8ee495 into main Aug 21, 2026
1 check passed
@eagmon
eagmon deleted the chore/zarr-3.3 branch August 21, 2026 07:07
eagmon added a commit that referenced this pull request Aug 21, 2026
#32 added bottleneck/fsspec to the [xarray] extra but did not re-lock, and #31
re-locked before those deps existed, so main's uv.lock was missing them
(`uv lock --check` failed). Re-lock to restore consistency.

Verified: `uv lock --check` passes; full suite 121 passed, 4 skipped.

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@eagmon eagmon mentioned this pull request Aug 21, 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