-
Notifications
You must be signed in to change notification settings - Fork 51
Zarr data types refactor compat round 2 #677
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
63ff6c9
e765e13
c60795a
b2d9549
a05d9e8
f172930
02dfa56
7dfeabe
b42022f
7bb6e6f
744fd49
35f94b5
9663d05
e1e54b3
915e4c3
ae3f871
8548086
01baf83
ad0be5c
de9c8fd
35cfddf
813ee16
f0739c1
77493a6
31941fb
64a513b
a17884d
93f7758
407f208
09d801d
9117116
3ebb731
bf104cb
b1961b6
b0bddf2
fc1bd34
925ffec
0d7ad60
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -343,12 +343,12 @@ def test_non_dimension_coordinates( | |
| for coord in ds.coords: | ||
| assert ds.coords[coord].attrs == roundtrip.coords[coord].attrs | ||
|
|
||
| @pytest.mark.xfail( | ||
| reason="Datetime and timedelta data types not yet supported by zarr-python 3.0" # https://github.com/zarr-developers/zarr-python/issues/2616 | ||
| ) | ||
| def test_datetime64_dtype_fill_value( | ||
| self, tmpdir, roundtrip_func, array_v3_metadata | ||
| ): | ||
| if roundtrip_func == roundtrip_as_in_memory_icechunk: | ||
| pytest.xfail(reason="xarray can't decode the ns datetime fill_value") | ||
|
|
||
|
Comment on lines
+349
to
+351
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As zarr-developers/zarr-python#2616 was closed, these xfails became xpasses! Except for this one case - roundtripping via Icechunk, which fails with an error inside xarray's zarr decoders... I don't really understand why this matters, but it should be treated as a separable issue from this PR.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Raised #686 to track this |
||
| chunks_dict = { | ||
| "0.0.0": {"path": "/foo.nc", "offset": 100, "length": 100}, | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,6 +11,7 @@ | |
| from xarray.conventions import encode_dataset_coordinates | ||
| from zarr.core.common import JSON | ||
| from zarr.core.metadata.v2 import ArrayV2Metadata | ||
| from zarr.dtype import parse_data_type | ||
|
|
||
| from virtualizarr.manifests import ManifestArray | ||
| from virtualizarr.manifests.manifest import join | ||
|
|
@@ -161,7 +162,9 @@ def variable_to_kerchunk_arr_refs(var: Variable, var_name: str) -> KerchunkArrRe | |
| array_v2_metadata = ArrayV2Metadata( | ||
| chunks=np_arr.shape, | ||
| shape=np_arr.shape, | ||
| dtype=np_arr.dtype, | ||
| dtype=parse_data_type( | ||
| np_arr.dtype, zarr_format=2 | ||
| ), # needed unless zarr-python fixes https://github.com/zarr-developers/zarr-python/issues/3253 | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This works around zarr-developers/zarr-python#3253 |
||
| order="C", | ||
| fill_value=None, | ||
| ) | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This comment explains why were are now able to simplify this: zarr-developers/zarr-python#2930 (comment)