From 01e75181ee904282e656ee01180c2d1d3e679239 Mon Sep 17 00:00:00 2001 From: TomNicholas Date: Thu, 24 Oct 2024 17:48:00 -0400 Subject: [PATCH 01/17] new blank whatsnew --- doc/whats-new.rst | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/doc/whats-new.rst b/doc/whats-new.rst index 9a451a836ad..18fae4e0151 100644 --- a/doc/whats-new.rst +++ b/doc/whats-new.rst @@ -14,6 +14,34 @@ What's New np.random.seed(123456) +.. _whats-new.2024.10.1: + +v.2024.10.1 (unreleased) +------------------------ + +New Features +~~~~~~~~~~~~ + + +Breaking changes +~~~~~~~~~~~~~~~~ + + +Deprecations +~~~~~~~~~~~~ + + +Bug fixes +~~~~~~~~~ + + +Documentation +~~~~~~~~~~~~~ + + +Internal Changes +~~~~~~~~~~~~~~~~ + .. _whats-new.2024.10.0: v2024.10.0 (Oct 24th, 2024) From 08cb041ab321ad61a2dee3436b16f1fc5a686135 Mon Sep 17 00:00:00 2001 From: Tom Nicholas Date: Fri, 27 Jun 2025 19:07:49 -0400 Subject: [PATCH 02/17] add test for store which explicitly does not support consolidated metadata --- xarray/tests/test_backends.py | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/xarray/tests/test_backends.py b/xarray/tests/test_backends.py index fe4c1684cbd..21553efc606 100644 --- a/xarray/tests/test_backends.py +++ b/xarray/tests/test_backends.py @@ -115,7 +115,9 @@ import zarr.codecs if has_zarr_v3: + from zarr.abc.store import Store from zarr.storage import MemoryStore as KVStore + from zarr.storage import WrapperStore ZARR_FORMATS = [2, 3] else: @@ -3744,6 +3746,29 @@ def test_chunk_key_encoding_v2(self) -> None: assert actual["var1"].encoding["chunks"] == (2, 2) +class NoConsolidatedMetadataSupportStore(WrapperStore[Store]): + """ + Store that explicitly does not support consolidated metadata. + + Useful as a proxy for stores like Icechunk, see https://github.com/zarr-developers/zarr-python/pull/3119. + """ + + supports_consolidated_metadata = False + + +@requires_zarr +class TestZarrNoConsolidatedMetadataSupport(ZarrBase): + @contextlib.contextmanager + def create_zarr_target(self): + # TODO the zarr version would need to be >3.08 for the supports_consolidated_metadata property to have any effect + if has_zarr_v3: + yield NoConsolidatedMetadataSupportStore( + zarr.storage.MemoryStore({}, read_only=False) + ) + else: + pytest.skip("requires zarr v3") + + @requires_zarr @pytest.mark.skipif( ON_WINDOWS, From 9cc91057d03f045a75d3785bd365fad93a23006f Mon Sep 17 00:00:00 2001 From: Tom Nicholas Date: Fri, 27 Jun 2025 19:08:03 -0400 Subject: [PATCH 03/17] check for consolidated metadata support --- xarray/backends/zarr.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/xarray/backends/zarr.py b/xarray/backends/zarr.py index 54ff419b2f2..4799aac5e66 100644 --- a/xarray/backends/zarr.py +++ b/xarray/backends/zarr.py @@ -1768,6 +1768,10 @@ def _get_open_params( else: missing_exc = zarr.errors.GroupNotFoundError + if _zarr_v3(): + if not store.supports_consolidated_metadata: + consolidated = consolidate_on_close = False + if consolidated in [None, True]: # open the root of the store, in case there is metadata consolidated there group = open_kwargs.pop("path") @@ -1825,6 +1829,7 @@ def _get_open_params( else: # this was the default for v2 and should apply to most existing Zarr data use_zarr_fill_value_as_mask = True + return ( zarr_group, consolidate_on_close, From c817869c0180a795aefff073e1f8f2935f23c6a0 Mon Sep 17 00:00:00 2001 From: Tom Nicholas Date: Sat, 28 Jun 2025 11:05:12 -0400 Subject: [PATCH 04/17] WrapperStore now only imported and used if zarr v3 --- xarray/tests/test_backends.py | 31 ++++++++++++++++++------------- 1 file changed, 18 insertions(+), 13 deletions(-) diff --git a/xarray/tests/test_backends.py b/xarray/tests/test_backends.py index 21553efc606..b9f20965ec7 100644 --- a/xarray/tests/test_backends.py +++ b/xarray/tests/test_backends.py @@ -128,6 +128,8 @@ ) except ImportError: KVStore = None # type: ignore[assignment,misc,unused-ignore] + + WrapperStore = None else: KVStore = None # type: ignore[assignment,misc,unused-ignore] ZARR_FORMATS = [] @@ -3746,27 +3748,30 @@ def test_chunk_key_encoding_v2(self) -> None: assert actual["var1"].encoding["chunks"] == (2, 2) -class NoConsolidatedMetadataSupportStore(WrapperStore[Store]): - """ - Store that explicitly does not support consolidated metadata. - - Useful as a proxy for stores like Icechunk, see https://github.com/zarr-developers/zarr-python/pull/3119. - """ - supports_consolidated_metadata = False @requires_zarr +@pytest.mark.skipif( + WrapperStore is None, reason="requires zarr v3" +) class TestZarrNoConsolidatedMetadataSupport(ZarrBase): @contextlib.contextmanager def create_zarr_target(self): + + class NoConsolidatedMetadataSupportStore(WrapperStore[Store]): + """ + Store that explicitly does not support consolidated metadata. + + Useful as a proxy for stores like Icechunk, see https://github.com/zarr-developers/zarr-python/pull/3119. + """ + + supports_consolidated_metadata = False + # TODO the zarr version would need to be >3.08 for the supports_consolidated_metadata property to have any effect - if has_zarr_v3: - yield NoConsolidatedMetadataSupportStore( - zarr.storage.MemoryStore({}, read_only=False) - ) - else: - pytest.skip("requires zarr v3") + yield NoConsolidatedMetadataSupportStore( + zarr.storage.MemoryStore({}, read_only=False) + ) @requires_zarr From 50cff3e0542cdcad7866ab4ef2ee1439a9359a18 Mon Sep 17 00:00:00 2001 From: Tom Nicholas Date: Sat, 28 Jun 2025 11:11:07 -0400 Subject: [PATCH 05/17] use dedicated decorator --- xarray/tests/test_backends.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/xarray/tests/test_backends.py b/xarray/tests/test_backends.py index b9f20965ec7..b3c78c0777a 100644 --- a/xarray/tests/test_backends.py +++ b/xarray/tests/test_backends.py @@ -88,6 +88,7 @@ requires_scipy, requires_scipy_or_netCDF4, requires_zarr, + requires_zarr_v3, ) from xarray.tests.test_coding_times import ( _ALL_CALENDARS, @@ -129,7 +130,7 @@ except ImportError: KVStore = None # type: ignore[assignment,misc,unused-ignore] - WrapperStore = None + WrapperStore = None # type: ignore[assignment,misc,unused-ignore] else: KVStore = None # type: ignore[assignment,misc,unused-ignore] ZARR_FORMATS = [] @@ -3751,10 +3752,7 @@ def test_chunk_key_encoding_v2(self) -> None: -@requires_zarr -@pytest.mark.skipif( - WrapperStore is None, reason="requires zarr v3" -) +@requires_zarr_v3 class TestZarrNoConsolidatedMetadataSupport(ZarrBase): @contextlib.contextmanager def create_zarr_target(self): From a47b2e2de4b86a2a8a4844b6dc661fc1dce253be Mon Sep 17 00:00:00 2001 From: Tom Nicholas Date: Sat, 28 Jun 2025 11:11:32 -0400 Subject: [PATCH 06/17] use getattr in case property not defined --- xarray/backends/zarr.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/xarray/backends/zarr.py b/xarray/backends/zarr.py index 4799aac5e66..6f959887be6 100644 --- a/xarray/backends/zarr.py +++ b/xarray/backends/zarr.py @@ -1769,7 +1769,8 @@ def _get_open_params( missing_exc = zarr.errors.GroupNotFoundError if _zarr_v3(): - if not store.supports_consolidated_metadata: + # zarr 3.0.8 and earlier did not support this property - it was effectively assumed true + if not getattr(store, "supports_consolidated_metadata", True): consolidated = consolidate_on_close = False if consolidated in [None, True]: From 76f16fe8baba0e5d97b809f838004e1c6e3f7956 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Sat, 28 Jun 2025 15:11:57 +0000 Subject: [PATCH 07/17] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- xarray/tests/test_backends.py | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/xarray/tests/test_backends.py b/xarray/tests/test_backends.py index b3c78c0777a..77c08dce55a 100644 --- a/xarray/tests/test_backends.py +++ b/xarray/tests/test_backends.py @@ -129,7 +129,7 @@ ) except ImportError: KVStore = None # type: ignore[assignment,misc,unused-ignore] - + WrapperStore = None # type: ignore[assignment,misc,unused-ignore] else: KVStore = None # type: ignore[assignment,misc,unused-ignore] @@ -3749,14 +3749,10 @@ def test_chunk_key_encoding_v2(self) -> None: assert actual["var1"].encoding["chunks"] == (2, 2) - - - @requires_zarr_v3 class TestZarrNoConsolidatedMetadataSupport(ZarrBase): @contextlib.contextmanager def create_zarr_target(self): - class NoConsolidatedMetadataSupportStore(WrapperStore[Store]): """ Store that explicitly does not support consolidated metadata. @@ -3765,7 +3761,7 @@ class NoConsolidatedMetadataSupportStore(WrapperStore[Store]): """ supports_consolidated_metadata = False - + # TODO the zarr version would need to be >3.08 for the supports_consolidated_metadata property to have any effect yield NoConsolidatedMetadataSupportStore( zarr.storage.MemoryStore({}, read_only=False) From e174d9c6a78c11aed3613b3f1973087217e58221 Mon Sep 17 00:00:00 2001 From: Tom Nicholas Date: Mon, 30 Jun 2025 11:34:33 -0400 Subject: [PATCH 08/17] implement with_read_only --- xarray/tests/test_backends.py | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/xarray/tests/test_backends.py b/xarray/tests/test_backends.py index b3c78c0777a..b92d7514c13 100644 --- a/xarray/tests/test_backends.py +++ b/xarray/tests/test_backends.py @@ -129,7 +129,7 @@ ) except ImportError: KVStore = None # type: ignore[assignment,misc,unused-ignore] - + WrapperStore = None # type: ignore[assignment,misc,unused-ignore] else: KVStore = None # type: ignore[assignment,misc,unused-ignore] @@ -3749,14 +3749,10 @@ def test_chunk_key_encoding_v2(self) -> None: assert actual["var1"].encoding["chunks"] == (2, 2) - - - @requires_zarr_v3 class TestZarrNoConsolidatedMetadataSupport(ZarrBase): @contextlib.contextmanager def create_zarr_target(self): - class NoConsolidatedMetadataSupportStore(WrapperStore[Store]): """ Store that explicitly does not support consolidated metadata. @@ -3765,7 +3761,23 @@ class NoConsolidatedMetadataSupportStore(WrapperStore[Store]): """ supports_consolidated_metadata = False - + + def __init__( + self, + store: Store, + *, + read_only: bool = False, + ) -> None: + self._store = store.with_read_only(read_only=read_only) + + def with_read_only( + self, read_only: bool = False + ) -> NoConsolidatedMetadataSupportStore: + return type(self)( + store=self._store, + read_only=read_only, + ) + # TODO the zarr version would need to be >3.08 for the supports_consolidated_metadata property to have any effect yield NoConsolidatedMetadataSupportStore( zarr.storage.MemoryStore({}, read_only=False) From 101b28074ea10d46e12d46d18758abc9e80ecfe7 Mon Sep 17 00:00:00 2001 From: Tom Nicholas Date: Wed, 2 Jul 2025 11:13:14 -0400 Subject: [PATCH 09/17] change test to not expect warning if consolidated metadata not supported --- xarray/tests/test_backends.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/xarray/tests/test_backends.py b/xarray/tests/test_backends.py index c2d0e511a49..f00a3a4388f 100644 --- a/xarray/tests/test_backends.py +++ b/xarray/tests/test_backends.py @@ -2405,12 +2405,13 @@ def test_read_non_consolidated_warning(self) -> None: self.save( expected, store_target=store, consolidated=False, **self.version_kwargs ) - with pytest.warns( - RuntimeWarning, - match="Failed to open Zarr store with consolidated", - ): - with xr.open_zarr(store, **self.version_kwargs) as ds: - assert_identical(ds, expected) + if getattr(store, "supports_consolidated_metadata", True): + with pytest.warns( + RuntimeWarning, + match="Failed to open Zarr store with consolidated", + ): + with xr.open_zarr(store, **self.version_kwargs) as ds: + assert_identical(ds, expected) def test_non_existent_store(self) -> None: with pytest.raises( From 26a402c7e2119cd081587cd48ee90411a2d52ce7 Mon Sep 17 00:00:00 2001 From: Tom Nicholas Date: Wed, 2 Jul 2025 11:27:29 -0400 Subject: [PATCH 10/17] move store definition outside of test --- xarray/tests/test_backends.py | 51 ++++++++++++++++++----------------- 1 file changed, 26 insertions(+), 25 deletions(-) diff --git a/xarray/tests/test_backends.py b/xarray/tests/test_backends.py index f00a3a4388f..3d748f15a85 100644 --- a/xarray/tests/test_backends.py +++ b/xarray/tests/test_backends.py @@ -3762,35 +3762,36 @@ def test_chunk_key_encoding_v2(self) -> None: assert actual["var1"].encoding["chunks"] == (2, 2) +class NoConsolidatedMetadataSupportStore(WrapperStore[Store]): + """ + Store that explicitly does not support consolidated metadata. + + Useful as a proxy for stores like Icechunk, see https://github.com/zarr-developers/zarr-python/pull/3119. + """ + + supports_consolidated_metadata = False + + def __init__( + self, + store: Store, + *, + read_only: bool = False, + ) -> None: + self._store = store.with_read_only(read_only=read_only) + + def with_read_only( + self, read_only: bool = False + ) -> NoConsolidatedMetadataSupportStore: + return type(self)( + store=self._store, + read_only=read_only, + ) + + @requires_zarr_v3 class TestZarrNoConsolidatedMetadataSupport(ZarrBase): @contextlib.contextmanager def create_zarr_target(self): - class NoConsolidatedMetadataSupportStore(WrapperStore[Store]): - """ - Store that explicitly does not support consolidated metadata. - - Useful as a proxy for stores like Icechunk, see https://github.com/zarr-developers/zarr-python/pull/3119. - """ - - supports_consolidated_metadata = False - - def __init__( - self, - store: Store, - *, - read_only: bool = False, - ) -> None: - self._store = store.with_read_only(read_only=read_only) - - def with_read_only( - self, read_only: bool = False - ) -> NoConsolidatedMetadataSupportStore: - return type(self)( - store=self._store, - read_only=read_only, - ) - # TODO the zarr version would need to be >3.08 for the supports_consolidated_metadata property to have any effect yield NoConsolidatedMetadataSupportStore( zarr.storage.MemoryStore({}, read_only=False) From 1ce9d87687aeea47d7cddb92291355c5ba2bcff6 Mon Sep 17 00:00:00 2001 From: Tom Nicholas Date: Wed, 2 Jul 2025 12:26:01 -0400 Subject: [PATCH 11/17] try setting Store=None when zarr-python v3 not installed --- xarray/tests/test_backends.py | 1 + 1 file changed, 1 insertion(+) diff --git a/xarray/tests/test_backends.py b/xarray/tests/test_backends.py index 3d748f15a85..6fb1216b543 100644 --- a/xarray/tests/test_backends.py +++ b/xarray/tests/test_backends.py @@ -131,6 +131,7 @@ except ImportError: KVStore = None # type: ignore[assignment,misc,unused-ignore] + Store = None # type: ignore[assignment,misc,unused-ignore] WrapperStore = None # type: ignore[assignment,misc,unused-ignore] else: KVStore = None # type: ignore[assignment,misc,unused-ignore] From ad4d79c2432b7671e3af0dd62847872e11bf0cc1 Mon Sep 17 00:00:00 2001 From: Tom Nicholas Date: Wed, 2 Jul 2025 12:42:53 -0400 Subject: [PATCH 12/17] remove Store type hint entirely to avoid import issues --- xarray/tests/test_backends.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/xarray/tests/test_backends.py b/xarray/tests/test_backends.py index 6fb1216b543..8e971ad4e9b 100644 --- a/xarray/tests/test_backends.py +++ b/xarray/tests/test_backends.py @@ -131,7 +131,6 @@ except ImportError: KVStore = None # type: ignore[assignment,misc,unused-ignore] - Store = None # type: ignore[assignment,misc,unused-ignore] WrapperStore = None # type: ignore[assignment,misc,unused-ignore] else: KVStore = None # type: ignore[assignment,misc,unused-ignore] @@ -3763,7 +3762,7 @@ def test_chunk_key_encoding_v2(self) -> None: assert actual["var1"].encoding["chunks"] == (2, 2) -class NoConsolidatedMetadataSupportStore(WrapperStore[Store]): +class NoConsolidatedMetadataSupportStore(WrapperStore): """ Store that explicitly does not support consolidated metadata. @@ -3774,7 +3773,7 @@ class NoConsolidatedMetadataSupportStore(WrapperStore[Store]): def __init__( self, - store: Store, + store, *, read_only: bool = False, ) -> None: From 09915dd3dfe31be22f701f4c79288e74031c8a64 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Wed, 2 Jul 2025 16:43:14 +0000 Subject: [PATCH 13/17] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- xarray/tests/test_backends.py | 1 - 1 file changed, 1 deletion(-) diff --git a/xarray/tests/test_backends.py b/xarray/tests/test_backends.py index 8e971ad4e9b..33ee715930a 100644 --- a/xarray/tests/test_backends.py +++ b/xarray/tests/test_backends.py @@ -117,7 +117,6 @@ import zarr.codecs if has_zarr_v3: - from zarr.abc.store import Store from zarr.storage import MemoryStore as KVStore from zarr.storage import WrapperStore From 8252f9f818d4b11dcb7fac0539c23bf4b5450913 Mon Sep 17 00:00:00 2001 From: Tom Nicholas Date: Wed, 2 Jul 2025 12:59:04 -0400 Subject: [PATCH 14/17] try WrapperStore = object --- xarray/tests/test_backends.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xarray/tests/test_backends.py b/xarray/tests/test_backends.py index 33ee715930a..dc450785cc2 100644 --- a/xarray/tests/test_backends.py +++ b/xarray/tests/test_backends.py @@ -130,7 +130,7 @@ except ImportError: KVStore = None # type: ignore[assignment,misc,unused-ignore] - WrapperStore = None # type: ignore[assignment,misc,unused-ignore] + WrapperStore = object # type: ignore[assignment,misc,unused-ignore] else: KVStore = None # type: ignore[assignment,misc,unused-ignore] ZARR_FORMATS = [] From e5508518b0879cc85d9290f3f16c3d08086ab357 Mon Sep 17 00:00:00 2001 From: Tom Nicholas Date: Wed, 2 Jul 2025 13:11:53 -0400 Subject: [PATCH 15/17] ensure WrapperStore is defined as something even if zarr isn't present at all --- xarray/tests/test_backends.py | 1 + 1 file changed, 1 insertion(+) diff --git a/xarray/tests/test_backends.py b/xarray/tests/test_backends.py index dc450785cc2..785b06a26fd 100644 --- a/xarray/tests/test_backends.py +++ b/xarray/tests/test_backends.py @@ -133,6 +133,7 @@ WrapperStore = object # type: ignore[assignment,misc,unused-ignore] else: KVStore = None # type: ignore[assignment,misc,unused-ignore] + WrapperStore = object # type: ignore[assignment,misc,unused-ignore] ZARR_FORMATS = [] From 586c7a2bca7a9591ad44eb35f4b83186fc867996 Mon Sep 17 00:00:00 2001 From: Tom Nicholas Date: Wed, 2 Jul 2025 15:27:38 -0400 Subject: [PATCH 16/17] release note --- doc/whats-new.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/doc/whats-new.rst b/doc/whats-new.rst index 833ac23d449..d3901244b5c 100644 --- a/doc/whats-new.rst +++ b/doc/whats-new.rst @@ -14,6 +14,7 @@ New Features ~~~~~~~~~~~~ - Expose :py:class:`~xarray.indexes.RangeIndex`, and :py:class:`~xarray.indexes.CoordinateTransformIndex` as public api under the ``xarray.indexes`` namespace. By `Deepak Cherian `_. +- Support zarr-python's new `.supports_consolidated_metadata` store property (:pull:`10457``), by Tom Nicholas `_. Breaking changes ~~~~~~~~~~~~~~~~ From 758eea44e5ccc338e972933eb28bc1833151b2f4 Mon Sep 17 00:00:00 2001 From: Tom Nicholas Date: Wed, 2 Jul 2025 15:29:41 -0400 Subject: [PATCH 17/17] correct RST syntax --- doc/whats-new.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/whats-new.rst b/doc/whats-new.rst index d3901244b5c..bc2d34cecd9 100644 --- a/doc/whats-new.rst +++ b/doc/whats-new.rst @@ -14,7 +14,7 @@ New Features ~~~~~~~~~~~~ - Expose :py:class:`~xarray.indexes.RangeIndex`, and :py:class:`~xarray.indexes.CoordinateTransformIndex` as public api under the ``xarray.indexes`` namespace. By `Deepak Cherian `_. -- Support zarr-python's new `.supports_consolidated_metadata` store property (:pull:`10457``), by Tom Nicholas `_. +- Support zarr-python's new ``.supports_consolidated_metadata`` store property (:pull:`10457``), by Tom Nicholas `_. Breaking changes ~~~~~~~~~~~~~~~~