Implement to_icechunk(region=...)#873
Implement to_icechunk(region=...)#873vladidobro wants to merge 9 commits intozarr-developers:mainfrom
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #873 +/- ##
==========================================
+ Coverage 89.00% 89.04% +0.03%
==========================================
Files 34 34
Lines 1956 1980 +24
==========================================
+ Hits 1741 1763 +22
- Misses 215 217 +2
🚀 New features to boost your workflow:
|
this doesn't look right EDIT: Ok I see it is because of the failed test probably |
virtualizarr/writers/icechunk.py
Outdated
| mode="r+", | ||
| ) | ||
| vds = xarray_store._validate_and_autodetect_region(vds) | ||
| region = xarray_store._write_region |
There was a problem hiding this comment.
I notice that this references "private" attributes of a class that's not from this package. Is that considered ok in VirtualiZarr?
There was a problem hiding this comment.
Hi,
in the referenced issue,
#308 (comment)
@TomNicholas said
Whether or not we vendor xarray kind of depends on how much of xarray we would have to vendor... Some of the internals are more stable than others. I would start out by not vendoring anything and we'll see how fragile that looks.
There was a problem hiding this comment.
Yeah it's not "okay", but arguably the lesser of two evils. VirtualiZarr as a whole is a slightly weird re-purposing of xarray.
If this is the only internal import from xarray then we have done better than I expected for this feature.
TomNicholas
left a comment
There was a problem hiding this comment.
This is looking good @vladidobro !
for more information, see https://pre-commit.ci
|
[Edited: please ignore. The bug turned out to be in my test harness. Writes outside the region of the array do in fact raise an exception.] |
| write_session.store, region={"y": "auto", "x": "auto"} | ||
| ) | ||
|
|
||
| @pytest.mark.xfail( |
There was a problem hiding this comment.
should I leave this test here? turns out I was unable write a datatree with xarray anyway
|
Hi, EDIT: actually let me just fix mypy, but otherwise I think it's ok |
docs/releases.rst