Dont write _ARRAY_DIMENSIONS to icechunk #286
Conversation
for more information, see https://pre-commit.ci
| @@ -144,8 +144,7 @@ def write_virtual_variable_to_icechunk( | |||
| # TODO it would be nice if we could assign directly to the .attrs property | |||
| for k, v in var.attrs.items(): | |||
| arr.attrs[k] = encode_zarr_attr_value(v) | |||
| arr.attrs["_ARRAY_DIMENSIONS"] = encode_zarr_attr_value(var.dims) | |||
There was a problem hiding this comment.
It's not necessary to write these because we already have dimension_names specified above in
arr = group.require_array(
name=name,
shape=zarray.shape,
chunk_shape=zarray.chunks,
dtype=encode_dtype(zarray.dtype),
codecs=zarray._v3_codec_pipeline(),
dimension_names=var.dims,
fill_value=zarray.fill_value,
# TODO fill_value?
)…olas/VirtualiZarr into icechunk_dimension_names
TomNicholas
temporarily deployed
to
test-release
GitHub Actions
Inactive
TomNicholas
temporarily deployed
to
test-release
GitHub Actions
Inactive
TomNicholas
temporarily deployed
to
test-release
GitHub Actions
Inactive
|
Getting the tests to pass required using the most recent version of xarray (for zarr v3 support). Not entirely sure why because the upstream CI was already using a zarrv3 compatibility branch of xarray. I assuming that branch did not have pydata/xarray#9669 in it. Anyway now that xarray 2024.10.0 is released we should just use that, so this PR should be merged immediately after #284. @mpiannucci any issues with this PR from your perspective? |
|
Looks good to me as long as xarray supports it now! |
Exactly right. I've removed the compat branch so we don't hit this again. |
docs/releases.rst