From b3eb68b93d8cd7707c5f34426804f15b5b5b1e6e Mon Sep 17 00:00:00 2001 From: Deepak Cherian Date: Fri, 26 Jan 2024 12:42:40 -0700 Subject: [PATCH 1/2] Fix negative slicing of Zarr arrays Closes #8252 Closes #3921 --- doc/whats-new.rst | 2 ++ xarray/backends/zarr.py | 9 ++++++++- xarray/tests/test_backends.py | 8 -------- 3 files changed, 10 insertions(+), 9 deletions(-) diff --git a/doc/whats-new.rst b/doc/whats-new.rst index 8865eb98481..e638cbdf2e7 100644 --- a/doc/whats-new.rst +++ b/doc/whats-new.rst @@ -44,6 +44,8 @@ Bug fixes By `Tom Nicholas `_. - Ensure :py:meth:`DataArray.unstack` works when wrapping array API-compliant classes. (:issue:`8666`, :pull:`8668`) By `Tom Nicholas `_. +- Fix negative slicing of Zarr arrays without dask installed. (:issue:`8252`) + By `Deepak Cherian `_. Documentation ~~~~~~~~~~~~~ diff --git a/xarray/backends/zarr.py b/xarray/backends/zarr.py index 469bbf4c339..316645c7f5d 100644 --- a/xarray/backends/zarr.py +++ b/xarray/backends/zarr.py @@ -86,10 +86,17 @@ def get_array(self): def _oindex(self, key): return self._array.oindex[key] + def _getitem(self, key): + return self._array[key] + def __getitem__(self, key): array = self._array if isinstance(key, indexing.BasicIndexer): - return array[key.tuple] + # this will convert negative slices to positive slices + # The latter are all that Zarr supports + return indexing.explicit_indexing_adapter( + key, array.shape, indexing.IndexingSupport.VECTORIZED, self._getitem + ) elif isinstance(key, indexing.VectorizedIndexer): return array.vindex[ indexing._arrayize_vectorized_indexer(key, self.shape).tuple diff --git a/xarray/tests/test_backends.py b/xarray/tests/test_backends.py index 85afbc1e147..d4539c9f770 100644 --- a/xarray/tests/test_backends.py +++ b/xarray/tests/test_backends.py @@ -2929,14 +2929,6 @@ def test_attributes(self, obj) -> None: with pytest.raises(TypeError, match=r"Invalid attribute in Dataset.attrs."): ds.to_zarr(store_target, **self.version_kwargs) - def test_vectorized_indexing_negative_step(self) -> None: - if not has_dask: - pytest.xfail( - reason="zarr without dask handles negative steps in slices incorrectly" - ) - - super().test_vectorized_indexing_negative_step() - @requires_zarr class TestZarrDictStore(ZarrBase): From 2027333628fda41d5d6d8e6f88d8a10b084aa8c3 Mon Sep 17 00:00:00 2001 From: Deepak Cherian Date: Fri, 26 Jan 2024 14:05:43 -0700 Subject: [PATCH 2/2] Cleanup --- xarray/backends/zarr.py | 23 ++++++++++------------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/xarray/backends/zarr.py b/xarray/backends/zarr.py index 316645c7f5d..8f9040477d9 100644 --- a/xarray/backends/zarr.py +++ b/xarray/backends/zarr.py @@ -86,26 +86,23 @@ def get_array(self): def _oindex(self, key): return self._array.oindex[key] + def _vindex(self, key): + return self._array.vindex[key] + def _getitem(self, key): return self._array[key] def __getitem__(self, key): array = self._array if isinstance(key, indexing.BasicIndexer): - # this will convert negative slices to positive slices - # The latter are all that Zarr supports - return indexing.explicit_indexing_adapter( - key, array.shape, indexing.IndexingSupport.VECTORIZED, self._getitem - ) + method = self._getitem elif isinstance(key, indexing.VectorizedIndexer): - return array.vindex[ - indexing._arrayize_vectorized_indexer(key, self.shape).tuple - ] - else: - assert isinstance(key, indexing.OuterIndexer) - return indexing.explicit_indexing_adapter( - key, array.shape, indexing.IndexingSupport.VECTORIZED, self._oindex - ) + method = self._vindex + elif isinstance(key, indexing.OuterIndexer): + method = self._oindex + return indexing.explicit_indexing_adapter( + key, array.shape, indexing.IndexingSupport.VECTORIZED, method + ) # if self.ndim == 0: # could possibly have a work-around for 0d data here