Skip to content

Fix ContainsGroupError when writing a DataTree with region/append_dim#1036

Merged
TomNicholas merged 2 commits into
zarr-developers:mainfrom
aaronspring:fix-datatree-region-containsgrouperror
Jul 16, 2026
Merged

Fix ContainsGroupError when writing a DataTree with region/append_dim#1036
TomNicholas merged 2 commits into
zarr-developers:mainfrom
aaronspring:fix-datatree-region-containsgrouperror

Conversation

@aaronspring

Copy link
Copy Markdown
Contributor

Problem

Writing a virtual DataTree with region or append_dim (forwarded to each node) always failed with zarr.errors.ContainsGroupError. virtual_datatree_to_icechunk created every node's group unconditionally with Group.from_store, but region/append_dim require the group and its arrays to already exist — so the write blew up before it ever got to writing the region/append.

This is the same class of failure as #1001, but for the DataTree path.

Fix

Mirror the branch that virtual_dataset_to_icechunk already had on main: open the existing group for region/append_dim, create it otherwise.

for store_path, vds in paths_and_virtual_datasets:
    if kwargs.get("append_dim") or kwargs.get("region"):
        group = Group.open(store=store_path, zarr_format=3)   # must already exist
    else:
        group = Group.from_store(store=store_path, zarr_format=3)  # create

Test

test_write_datatree_region was marked xfail with the reason "This doesn't work in xarray either, maybe not even intended?" — but that was a misdiagnosis. On main, running it with --runxfail fails with ContainsGroupError at the group-creation step, not in any xarray region logic. With this fix the test runs to completion and its assertions pass, so the xfail marker is removed.

🤖 Generated with Claude Code

virtual_datatree_to_icechunk unconditionally created each node's group
with Group.from_store, so forwarding region or append_dim (which require
the group and its arrays to already exist) always failed with
ContainsGroupError. Mirror the branch virtual_dataset_to_icechunk
already had: open the existing group for region/append_dim, create it
otherwise.

Removes the now-passing xfail on test_write_datatree_region, whose
stated reason (an xarray region limitation) was a misdiagnosis of this
ContainsGroupError.

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

codecov Bot commented Jul 8, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.33%. Comparing base (be604ed) to head (148ea28).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1036   +/-   ##
=======================================
  Coverage   90.32%   90.33%           
=======================================
  Files          37       37           
  Lines        2233     2235    +2     
=======================================
+ Hits         2017     2019    +2     
  Misses        216      216           
Files with missing lines Coverage Δ
virtualizarr/writers/icechunk.py 91.66% <100.00%> (+0.10%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@aaronspring

Copy link
Copy Markdown
Contributor Author

CI note: the 3 red build jobs (test-py312, test-py313, minimum-versions) fail only on test_parsers/test_grib.py (test_grib_virtual_dataset, test_grib_only_variables, test_grib_chunk_decodes_via_codec) — the GRIB/gribberish parser returning empty data_vars / a NodeTypeValidationError. These are pre-existing and unrelated to this PR, which touches only the icechunk DataTree writer; all 550 other tests pass. The same 3 failures appear on #1034 (a pre-commit.ci autoupdate that changes no source), #1032, #1033, and on scheduled main runs.

@TomNicholas
TomNicholas enabled auto-merge (squash) July 15, 2026 23:57
@TomNicholas
TomNicholas merged commit a8eb06d into zarr-developers:main Jul 16, 2026
17 checks passed
@TomNicholas

Copy link
Copy Markdown
Member

Thanks @aaronspring !

TomNicholas added a commit to aaronspring/VirtualiZarr that referenced this pull request Jul 16, 2026
The merge of main (zarr-developers#1036) into this branch resolved the group-creation
loop in favor of zarr-developers#1036's Group.open/from_store branch, discarding the
open_group(mode=open_mode) call this PR introduced. open_mode was
computed but never used, so mode="a" against an existing group raised
ContainsGroupError. Restore the unified open_group call, which subsumes
zarr-developers#1036's append_dim/region branch via _resolve_mode returning "r+".
TomNicholas added a commit that referenced this pull request Jul 16, 2026
* Open existing groups when writing a DataTree with region/append_dim

virtual_datatree_to_icechunk unconditionally created each node's group
with Group.from_store, so forwarding region or append_dim (which require
the group and its arrays to already exist) always failed with
ContainsGroupError. Mirror the branch virtual_dataset_to_icechunk
already had: open the existing group for region/append_dim, create it
otherwise.

Removes the now-passing xfail on test_write_datatree_region, whose
stated reason (an xarray region limitation) was a misdiagnosis of this
ContainsGroupError.

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

* Surface mode in to_icechunk, keeping current defaults

Add a mode parameter ("w", "w-", "a") to vds.vz.to_icechunk and
dt.vz.to_icechunk. The default (None) keeps the existing behaviour of
raising ContainsGroupError when the target group already exists, while
mode="a" opens the existing group so datasets can e.g. be split across
commits by variable, and mode="w" overwrites existing contents.

A _resolve_mode helper validates mode and resolves it to the effective
zarr group-open mode ("r+" when append_dim/region require an existing
group), so both writers open groups through a single zarr.open_group
call.

Closes #1001.

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

* Fix merge botch: honor mode in virtual_datatree_to_icechunk loop

The merge of main (#1036) into this branch resolved the group-creation
loop in favor of #1036's Group.open/from_store branch, discarding the
open_group(mode=open_mode) call this PR introduced. open_mode was
computed but never used, so mode="a" against an existing group raised
ContainsGroupError. Restore the unified open_group call, which subsumes
#1036's append_dim/region branch via _resolve_mode returning "r+".

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Co-authored-by: Tom Nicholas <tom@earthmover.io>
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