Skip to content

Updates for Zarr 3 Dtypes #10456

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

Open
wants to merge 20 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions .github/workflows/upstream-dev-ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,7 @@ jobs:
name: detect upstream-dev ci trigger
runs-on: ubuntu-latest
if: |
github.repository == 'pydata/xarray'
&& (github.event_name == 'push' || github.event_name == 'pull_request')
github.event_name == 'push' || github.event_name == 'pull_request'
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An attempt to get upstream tests to run on this PR

outputs:
triggered: ${{ steps.detect-trigger.outputs.trigger-found }}
steps:
Expand Down
4 changes: 3 additions & 1 deletion xarray/backends/zarr.py
Original file line number Diff line number Diff line change
Expand Up @@ -820,7 +820,9 @@ def open_store_variable(self, name):
attributes["_FillValue"], zarr_array.dtype
)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this code is correct and should be restored


return Variable(dimensions, data, attributes, encoding)
variable = Variable(dimensions, data, attributes, encoding)

return variable
Comment on lines +823 to +825
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
variable = Variable(dimensions, data, attributes, encoding)
return variable
return Variable(dimensions, data, attributes, encoding)


def get_variables(self):
return FrozenDict((k, self.open_store_variable(k)) for k in self.array_keys())
Expand Down
14 changes: 14 additions & 0 deletions xarray/tests/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,20 @@ def _importorskip(
has_rasterio, requires_rasterio = _importorskip("rasterio")
has_zarr, requires_zarr = _importorskip("zarr")
has_zarr_v3, requires_zarr_v3 = _importorskip("zarr", "3.0.0")
has_zarr_v3_dtypes, requires_zarr_v3_dtypes = _importorskip("zarr", "3.1.0")
if has_zarr_v3:
import zarr

# manual update by checking attrs for now
# TODO: use version specifier
# installing from git main is giving me a lower version than the
# most recently released zarr
has_zarr_v3_dtypes = hasattr(zarr.core, "dtype")

requires_zarr_v3_dtypes = pytest.mark.skipif(
not has_zarr_v3_dtypes, reason="requires zarr>3.1.0"
)

Comment on lines +134 to +146
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can this be removed now?

has_fsspec, requires_fsspec = _importorskip("fsspec")
has_iris, requires_iris = _importorskip("iris")
has_numbagg, requires_numbagg = _importorskip("numbagg")
Expand Down
12 changes: 10 additions & 2 deletions xarray/tests/test_backends.py
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@
has_scipy,
has_zarr,
has_zarr_v3,
has_zarr_v3_dtypes,
mock,
network,
requires_cftime,
Expand Down Expand Up @@ -2437,7 +2438,7 @@ def test_read_non_consolidated_warning(self) -> None:
def test_non_existent_store(self) -> None:
with pytest.raises(
FileNotFoundError,
match="(No such file or directory|Unable to find group|No group found)",
match="(No such file or directory|Unable to find group|No group found in store)",
):
xr.open_zarr(f"{uuid.uuid4()}")

Expand Down Expand Up @@ -2519,6 +2520,7 @@ def test_manual_chunk(self) -> None:
assert_identical(actual.load(), auto.load())

@requires_dask
@pytest.mark.filterwarnings("ignore:.*does not have a Zarr V3 specification.*")
def test_warning_on_bad_chunks(self) -> None:
original = create_test_data().chunk({"dim1": 4, "dim2": 3, "dim3": 3})

Expand Down Expand Up @@ -2927,7 +2929,9 @@ def test_append_with_existing_encoding_raises(self) -> None:

@pytest.mark.parametrize("dtype", ["U", "S"])
def test_append_string_length_mismatch_raises(self, dtype) -> None:
skip_if_zarr_format_3("This actually works fine with Zarr format 3")
if has_zarr_v3 and not has_zarr_v3_dtypes:
skip_if_zarr_format_3("This actually works fine with Zarr format 3")

ds, ds_to_append = create_append_string_length_mismatch_test_data(dtype)
with self.create_zarr_target() as store_target:
ds.to_zarr(store_target, mode="w", **self.version_kwargs)
Expand All @@ -2940,8 +2944,12 @@ def test_append_string_length_mismatch_raises(self, dtype) -> None:
def test_append_string_length_mismatch_works(self, dtype) -> None:
skip_if_zarr_format_2("This doesn't work with Zarr format 2")
# ...but it probably would if we used object dtype
if has_zarr_v3_dtypes:
pytest.skip("This works on pre ZDtype Zarr-Python, but fails after.")

ds, ds_to_append = create_append_string_length_mismatch_test_data(dtype)
expected = xr.concat([ds, ds_to_append], dim="time")

with self.create_zarr_target() as store_target:
ds.to_zarr(store_target, mode="w", **self.version_kwargs)
ds_to_append.to_zarr(store_target, append_dim="time", **self.version_kwargs)
Expand Down
Loading