Surface mode in vz.to_icechunk, keeping current defaults#1035
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1035 +/- ##
==========================================
+ Coverage 90.33% 90.36% +0.03%
==========================================
Files 37 37
Lines 2235 2242 +7
==========================================
+ Hits 2019 2026 +7
Misses 216 216
🚀 New features to boost your workflow:
|
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>
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 zarr-developers#1001.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
397e065 to
d787844
Compare
|
Split the DataTree |
|
CI note: the 3 red build jobs ( |
|
|
||
| - ``"w-"``: create the group, raising a ``ContainsGroupError`` if it already exists. | ||
| - ``"w"``: create the group, overwriting any existing contents at that path. | ||
| - ``"a"``: open the group if it exists (keeping existing arrays), otherwise create it. |
There was a problem hiding this comment.
should I also allow r+ if region and a-?
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+".
|
Thank you very much @aaronspring ! |
Closes #1001.
Exposes
modeinvz.to_icechunkclaude
What this does
Adds a
modeparameter tovds.vz.to_icechunkanddt.vz.to_icechunk(and the underlyingvirtual_dataset_to_icechunk/virtual_datatree_to_icechunkwriters) controlling how a pre-existing group at the target path is handled:"w-": create the group, raisingContainsGroupErrorif it already exists — the previous and still-default behaviour (mode=None)."w": create the group, overwriting any existing contents at that path."a": open the group if it exists (keeping existing arrays), otherwise create it. This enables e.g. splitting a large virtual dataset across commits by variable, and writing a parent group after a child group — both scenarios fromvz.to_icechunkthrowsContainsGroupErrorwhen group already exists and doesn't surfacemode#1001.mode="w"/"w-"combined withappend_dimorregionraises aValueError, since those require opening an existing group.Implementation
Internally, a
_resolve_modehelper validatesmodeand resolves it to the effective zarr group-open mode ("r+"whenappend_dim/regionare given), and both writers now open groups through a single unconditionalzarr.open_group(...)call instead of branching betweenGroup.openandGroup.from_store.A side effect of that unification: writing a virtual
DataTreewithregion/append_dimforwarded to each node previously always failed withContainsGroupError(every node group was unconditionally created); existing groups are now opened instead, and the previously-xfailingtest_write_datatree_regionpasses, so its xfail marker is removed.Tests & docs
"w-"still raising on existing groups (regression guard), variable-split across commits withmode="a"(the exactvz.to_icechunkthrowsContainsGroupErrorwhen group already exists and doesn't surfacemode#1001 workflow), parent-group-after-child withmode="a",mode="w"overwrite, invalid mode values,mode×append_dimconflicts, and aDataTreemode="a"write.🤖 Generated with Claude Code