From 975616588923c858d9381af60c9fc265fe21d285 Mon Sep 17 00:00:00 2001 From: Max Jones <14077947+maxrjones@users.noreply.github.com> Date: Tue, 17 Jun 2025 19:34:05 -0400 Subject: [PATCH 01/10] Add with_read_only() convenience method to store --- src/zarr/abc/store.py | 16 +++++++++++++ src/zarr/storage/_memory.py | 7 ++++++ src/zarr/testing/store.py | 8 +++++++ tests/test_store/test_memory.py | 40 +++++++++++++++++++++++++++++++++ 4 files changed, 71 insertions(+) diff --git a/src/zarr/abc/store.py b/src/zarr/abc/store.py index db4dee8cdd..3f87d2aae4 100644 --- a/src/zarr/abc/store.py +++ b/src/zarr/abc/store.py @@ -83,6 +83,22 @@ async def open(cls, *args: Any, **kwargs: Any) -> Self: await store._open() return store + def with_read_only(self, read_only: bool = False) -> Store: + """ + Return a new store of the same type pointing to the same location with the specified read_only state. + The returned Store is not automatically opened. + + Parameters + ---------- + read_only + If True, the store will be created in read-only mode. Defaults to False. + + Returns + ------- + A new store of the same type with the new read only attribute. + """ + raise NotImplementedError("with_read_only is not implemented for this store type.") + def __enter__(self) -> Self: """Enter a context manager that will close the store upon exiting.""" return self diff --git a/src/zarr/storage/_memory.py b/src/zarr/storage/_memory.py index ea25f82a3b..0dc6f13236 100644 --- a/src/zarr/storage/_memory.py +++ b/src/zarr/storage/_memory.py @@ -54,6 +54,13 @@ def __init__( store_dict = {} self._store_dict = store_dict + def with_read_only(self, read_only: bool = False) -> MemoryStore: + # docstring inherited + return type(self)( + store_dict=self._store_dict, + read_only=read_only, + ) + async def clear(self) -> None: # docstring inherited self._store_dict.clear() diff --git a/src/zarr/testing/store.py b/src/zarr/testing/store.py index 0e73599791..23061df714 100644 --- a/src/zarr/testing/store.py +++ b/src/zarr/testing/store.py @@ -149,6 +149,14 @@ async def test_read_only_store_raises(self, open_kwargs: dict[str, Any]) -> None ): await store.delete("foo") + async def test_with_read_only_store(self, open_kwargs: dict[str, Any]) -> None: + kwargs = {**open_kwargs, "read_only": True} + store = await self.store_cls.open(**kwargs) + with pytest.raises( + NotImplementedError, match="with_read_only is not implemented for this store type." + ): + store.with_read_only(read_only=False) + @pytest.mark.parametrize("key", ["c/0", "foo/c/0.0", "foo/0/0"]) @pytest.mark.parametrize( ("data", "byte_range"), diff --git a/tests/test_store/test_memory.py b/tests/test_store/test_memory.py index 4fc3f6e698..bf2fa391b2 100644 --- a/tests/test_store/test_memory.py +++ b/tests/test_store/test_memory.py @@ -78,6 +78,46 @@ async def test_deterministic_size( np.testing.assert_array_equal(a[:3], 1) np.testing.assert_array_equal(a[3:], 0) + async def test_with_read_only_store(self, open_kwargs: dict[str, Any]) -> None: + kwargs = {**open_kwargs, "read_only": True} + store = await self.store_cls.open(**kwargs) + assert store.read_only + + # Test that you cannot write to a read-only store + with pytest.raises( + ValueError, match="store was opened in read-only mode and does not support writing" + ): + await store.set("foo", self.buffer_cls.from_bytes(b"bar")) + + # Test that you can write to a copy that is not read-only + writer = store.with_read_only(read_only=False) + assert not writer._is_open + assert not writer.read_only + await writer.set("foo", self.buffer_cls.from_bytes(b"bar")) + await writer.delete("foo") + + # Test that you cannot write to the original store + assert store.read_only + with pytest.raises( + ValueError, match="store was opened in read-only mode and does not support writing" + ): + await store.set("foo", self.buffer_cls.from_bytes(b"bar")) + with pytest.raises( + ValueError, match="store was opened in read-only mode and does not support writing" + ): + await store.delete("foo") + # Test that you cannot write to a read-only store copy + reader = store.with_read_only(read_only=True) + assert reader.read_only + with pytest.raises( + ValueError, match="store was opened in read-only mode and does not support writing" + ): + await reader.set("foo", self.buffer_cls.from_bytes(b"bar")) + with pytest.raises( + ValueError, match="store was opened in read-only mode and does not support writing" + ): + await reader.delete("foo") + # TODO: fix this warning @pytest.mark.filterwarnings("ignore:Unclosed client session:ResourceWarning") From 83c97ac91257f5546ac7063d8477f049b9887290 Mon Sep 17 00:00:00 2001 From: Max Jones <14077947+maxrjones@users.noreply.github.com> Date: Wed, 18 Jun 2025 10:39:12 -0400 Subject: [PATCH 02/10] Consolidate test --- src/zarr/testing/store.py | 47 +++++++++++++++++++++++++++++++-- tests/test_store/test_memory.py | 40 ---------------------------- 2 files changed, 45 insertions(+), 42 deletions(-) diff --git a/src/zarr/testing/store.py b/src/zarr/testing/store.py index 23061df714..124c39282f 100644 --- a/src/zarr/testing/store.py +++ b/src/zarr/testing/store.py @@ -152,10 +152,53 @@ async def test_read_only_store_raises(self, open_kwargs: dict[str, Any]) -> None async def test_with_read_only_store(self, open_kwargs: dict[str, Any]) -> None: kwargs = {**open_kwargs, "read_only": True} store = await self.store_cls.open(**kwargs) + assert store.read_only + + # Test that you cannot write to a read-only store with pytest.raises( - NotImplementedError, match="with_read_only is not implemented for this store type." + ValueError, match="store was opened in read-only mode and does not support writing" ): - store.with_read_only(read_only=False) + await store.set("foo", self.buffer_cls.from_bytes(b"bar")) + + # Check if the store implements with_read_only + try: + writer = store.with_read_only(read_only=False) + + # Test that you can write to a new store copy + assert not writer._is_open + assert not writer.read_only + await writer.set("foo", self.buffer_cls.from_bytes(b"bar")) + await writer.delete("foo") + + # Test that you cannot write to the original store + assert store.read_only + with pytest.raises( + ValueError, match="store was opened in read-only mode and does not support writing" + ): + await store.set("foo", self.buffer_cls.from_bytes(b"bar")) + with pytest.raises( + ValueError, match="store was opened in read-only mode and does not support writing" + ): + await store.delete("foo") + + # Test that you cannot write to a read-only store copy + reader = store.with_read_only(read_only=True) + assert reader.read_only + with pytest.raises( + ValueError, match="store was opened in read-only mode and does not support writing" + ): + await reader.set("foo", self.buffer_cls.from_bytes(b"bar")) + with pytest.raises( + ValueError, match="store was opened in read-only mode and does not support writing" + ): + await reader.delete("foo") + + except NotImplementedError: + # Test that stores that do not implement with_read_only raise NotImplementedError with the correct message + with pytest.raises( + NotImplementedError, match="with_read_only is not implemented for this store type." + ): + store.with_read_only(read_only=False) @pytest.mark.parametrize("key", ["c/0", "foo/c/0.0", "foo/0/0"]) @pytest.mark.parametrize( diff --git a/tests/test_store/test_memory.py b/tests/test_store/test_memory.py index bf2fa391b2..4fc3f6e698 100644 --- a/tests/test_store/test_memory.py +++ b/tests/test_store/test_memory.py @@ -78,46 +78,6 @@ async def test_deterministic_size( np.testing.assert_array_equal(a[:3], 1) np.testing.assert_array_equal(a[3:], 0) - async def test_with_read_only_store(self, open_kwargs: dict[str, Any]) -> None: - kwargs = {**open_kwargs, "read_only": True} - store = await self.store_cls.open(**kwargs) - assert store.read_only - - # Test that you cannot write to a read-only store - with pytest.raises( - ValueError, match="store was opened in read-only mode and does not support writing" - ): - await store.set("foo", self.buffer_cls.from_bytes(b"bar")) - - # Test that you can write to a copy that is not read-only - writer = store.with_read_only(read_only=False) - assert not writer._is_open - assert not writer.read_only - await writer.set("foo", self.buffer_cls.from_bytes(b"bar")) - await writer.delete("foo") - - # Test that you cannot write to the original store - assert store.read_only - with pytest.raises( - ValueError, match="store was opened in read-only mode and does not support writing" - ): - await store.set("foo", self.buffer_cls.from_bytes(b"bar")) - with pytest.raises( - ValueError, match="store was opened in read-only mode and does not support writing" - ): - await store.delete("foo") - # Test that you cannot write to a read-only store copy - reader = store.with_read_only(read_only=True) - assert reader.read_only - with pytest.raises( - ValueError, match="store was opened in read-only mode and does not support writing" - ): - await reader.set("foo", self.buffer_cls.from_bytes(b"bar")) - with pytest.raises( - ValueError, match="store was opened in read-only mode and does not support writing" - ): - await reader.delete("foo") - # TODO: fix this warning @pytest.mark.filterwarnings("ignore:Unclosed client session:ResourceWarning") From b0b5b094b6503837a54236b49ee4bec19277174d Mon Sep 17 00:00:00 2001 From: Max Jones <14077947+maxrjones@users.noreply.github.com> Date: Wed, 18 Jun 2025 11:55:19 -0400 Subject: [PATCH 03/10] Add with_read_only to FsspecStore --- src/zarr/storage/_fsspec.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/zarr/storage/_fsspec.py b/src/zarr/storage/_fsspec.py index ba673056a3..4f6929456e 100644 --- a/src/zarr/storage/_fsspec.py +++ b/src/zarr/storage/_fsspec.py @@ -122,6 +122,7 @@ class FsspecStore(Store): fs: AsyncFileSystem allowed_exceptions: tuple[type[Exception], ...] + path: str def __init__( self, @@ -258,6 +259,15 @@ def from_url( return cls(fs=fs, path=path, read_only=read_only, allowed_exceptions=allowed_exceptions) + def with_read_only(self, read_only: bool = False) -> FsspecStore: + # docstring inherited + return type(self)( + fs=self.fs, + path=self.path, + allowed_exceptions=self.allowed_exceptions, + read_only=read_only, + ) + async def clear(self) -> None: # docstring inherited try: From 907585aac400d48e3ecdd8c76a91e6d5b5da6f74 Mon Sep 17 00:00:00 2001 From: Max Jones <14077947+maxrjones@users.noreply.github.com> Date: Wed, 18 Jun 2025 11:58:40 -0400 Subject: [PATCH 04/10] Add with_read_only to LocalStore --- src/zarr/storage/_local.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/zarr/storage/_local.py b/src/zarr/storage/_local.py index 15b043b1dc..43e585415d 100644 --- a/src/zarr/storage/_local.py +++ b/src/zarr/storage/_local.py @@ -102,6 +102,13 @@ def __init__(self, root: Path | str, *, read_only: bool = False) -> None: ) self.root = root + def with_read_only(self, read_only: bool = False) -> LocalStore: + # docstring inherited + return type(self)( + root=self.root, + read_only=read_only, + ) + async def _open(self) -> None: if not self.read_only: self.root.mkdir(parents=True, exist_ok=True) From 65812c8f47cbfe7ffbe83f550e7f921b0d835701 Mon Sep 17 00:00:00 2001 From: Max Jones <14077947+maxrjones@users.noreply.github.com> Date: Wed, 18 Jun 2025 12:01:43 -0400 Subject: [PATCH 05/10] Add with_read_only to ObjectStore --- src/zarr/storage/_obstore.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/zarr/storage/_obstore.py b/src/zarr/storage/_obstore.py index c048721cae..047ed07fbb 100644 --- a/src/zarr/storage/_obstore.py +++ b/src/zarr/storage/_obstore.py @@ -69,6 +69,13 @@ def __init__(self, store: _UpstreamObjectStore, *, read_only: bool = False) -> N super().__init__(read_only=read_only) self.store = store + def with_read_only(self, read_only: bool = False) -> ObjectStore: + # docstring inherited + return type(self)( + store=self.store, + read_only=read_only, + ) + def __str__(self) -> str: return f"object_store://{self.store}" From 2500b0ad932b755b5624d5973d40b0c8a57df0fd Mon Sep 17 00:00:00 2001 From: Max Jones <14077947+maxrjones@users.noreply.github.com> Date: Wed, 18 Jun 2025 12:04:58 -0400 Subject: [PATCH 06/10] Add changelog --- changes/3138.feature.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 changes/3138.feature.rst diff --git a/changes/3138.feature.rst b/changes/3138.feature.rst new file mode 100644 index 0000000000..ecd339bf9c --- /dev/null +++ b/changes/3138.feature.rst @@ -0,0 +1 @@ +Adds a `with_read_only` convenience method to the `Store` abstract base class (raises `NotImplementedError`) and implementations to the `MemoryStore`, `ObjectStore`, `LocalStore`, and `FsspecStore` classes. \ No newline at end of file From 11abad120bc5f5237f49c3824b5fe6ef0ecd75d5 Mon Sep 17 00:00:00 2001 From: Max Jones <14077947+maxrjones@users.noreply.github.com> Date: Wed, 18 Jun 2025 13:41:59 -0400 Subject: [PATCH 07/10] Include type in NotImplementedError --- src/zarr/abc/store.py | 4 +++- src/zarr/testing/store.py | 3 ++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/zarr/abc/store.py b/src/zarr/abc/store.py index 3f87d2aae4..26df5ade93 100644 --- a/src/zarr/abc/store.py +++ b/src/zarr/abc/store.py @@ -97,7 +97,9 @@ def with_read_only(self, read_only: bool = False) -> Store: ------- A new store of the same type with the new read only attribute. """ - raise NotImplementedError("with_read_only is not implemented for this store type.") + raise NotImplementedError( + f"with_read_only is not implemented for the {type(self)} store type." + ) def __enter__(self) -> Self: """Enter a context manager that will close the store upon exiting.""" diff --git a/src/zarr/testing/store.py b/src/zarr/testing/store.py index 124c39282f..4c2f3eb0ee 100644 --- a/src/zarr/testing/store.py +++ b/src/zarr/testing/store.py @@ -196,7 +196,8 @@ async def test_with_read_only_store(self, open_kwargs: dict[str, Any]) -> None: except NotImplementedError: # Test that stores that do not implement with_read_only raise NotImplementedError with the correct message with pytest.raises( - NotImplementedError, match="with_read_only is not implemented for this store type." + NotImplementedError, + match=f"with_read_only is not implemented for the {type(store)} store type.", ): store.with_read_only(read_only=False) From 0b7414dbbc9de84430d7ba79e78f83ab3ef726d0 Mon Sep 17 00:00:00 2001 From: Max Jones <14077947+maxrjones@users.noreply.github.com> Date: Wed, 18 Jun 2025 17:04:32 -0400 Subject: [PATCH 08/10] Apply suggestions from code review Co-authored-by: David Stansby --- src/zarr/abc/store.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/zarr/abc/store.py b/src/zarr/abc/store.py index 26df5ade93..050d3a4f99 100644 --- a/src/zarr/abc/store.py +++ b/src/zarr/abc/store.py @@ -85,8 +85,11 @@ async def open(cls, *args: Any, **kwargs: Any) -> Self: def with_read_only(self, read_only: bool = False) -> Store: """ - Return a new store of the same type pointing to the same location with the specified read_only state. - The returned Store is not automatically opened. + Return a new store with a new read_only setting. + + The new store points to the same location with the specified new read_only state. + The returned Store is not automatically opened, and this store is + not automatically closed. Parameters ---------- From 2789f4c12eac224f8ca9e41e761be4ab37006e57 Mon Sep 17 00:00:00 2001 From: Max Jones <14077947+maxrjones@users.noreply.github.com> Date: Wed, 18 Jun 2025 17:21:52 -0400 Subject: [PATCH 09/10] Lint --- src/zarr/abc/store.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/zarr/abc/store.py b/src/zarr/abc/store.py index 050d3a4f99..1fbdb3146c 100644 --- a/src/zarr/abc/store.py +++ b/src/zarr/abc/store.py @@ -86,7 +86,7 @@ async def open(cls, *args: Any, **kwargs: Any) -> Self: def with_read_only(self, read_only: bool = False) -> Store: """ Return a new store with a new read_only setting. - + The new store points to the same location with the specified new read_only state. The returned Store is not automatically opened, and this store is not automatically closed. From dff7034c0a30a30ad9050da0a90b1cf53a531cb6 Mon Sep 17 00:00:00 2001 From: Max Jones <14077947+maxrjones@users.noreply.github.com> Date: Wed, 18 Jun 2025 17:29:27 -0400 Subject: [PATCH 10/10] Apply suggestion to shorten try block --- src/zarr/testing/store.py | 60 +++++++++++++++++++-------------------- 1 file changed, 30 insertions(+), 30 deletions(-) diff --git a/src/zarr/testing/store.py b/src/zarr/testing/store.py index 4c2f3eb0ee..970329f393 100644 --- a/src/zarr/testing/store.py +++ b/src/zarr/testing/store.py @@ -163,36 +163,6 @@ async def test_with_read_only_store(self, open_kwargs: dict[str, Any]) -> None: # Check if the store implements with_read_only try: writer = store.with_read_only(read_only=False) - - # Test that you can write to a new store copy - assert not writer._is_open - assert not writer.read_only - await writer.set("foo", self.buffer_cls.from_bytes(b"bar")) - await writer.delete("foo") - - # Test that you cannot write to the original store - assert store.read_only - with pytest.raises( - ValueError, match="store was opened in read-only mode and does not support writing" - ): - await store.set("foo", self.buffer_cls.from_bytes(b"bar")) - with pytest.raises( - ValueError, match="store was opened in read-only mode and does not support writing" - ): - await store.delete("foo") - - # Test that you cannot write to a read-only store copy - reader = store.with_read_only(read_only=True) - assert reader.read_only - with pytest.raises( - ValueError, match="store was opened in read-only mode and does not support writing" - ): - await reader.set("foo", self.buffer_cls.from_bytes(b"bar")) - with pytest.raises( - ValueError, match="store was opened in read-only mode and does not support writing" - ): - await reader.delete("foo") - except NotImplementedError: # Test that stores that do not implement with_read_only raise NotImplementedError with the correct message with pytest.raises( @@ -200,6 +170,36 @@ async def test_with_read_only_store(self, open_kwargs: dict[str, Any]) -> None: match=f"with_read_only is not implemented for the {type(store)} store type.", ): store.with_read_only(read_only=False) + return + + # Test that you can write to a new store copy + assert not writer._is_open + assert not writer.read_only + await writer.set("foo", self.buffer_cls.from_bytes(b"bar")) + await writer.delete("foo") + + # Test that you cannot write to the original store + assert store.read_only + with pytest.raises( + ValueError, match="store was opened in read-only mode and does not support writing" + ): + await store.set("foo", self.buffer_cls.from_bytes(b"bar")) + with pytest.raises( + ValueError, match="store was opened in read-only mode and does not support writing" + ): + await store.delete("foo") + + # Test that you cannot write to a read-only store copy + reader = store.with_read_only(read_only=True) + assert reader.read_only + with pytest.raises( + ValueError, match="store was opened in read-only mode and does not support writing" + ): + await reader.set("foo", self.buffer_cls.from_bytes(b"bar")) + with pytest.raises( + ValueError, match="store was opened in read-only mode and does not support writing" + ): + await reader.delete("foo") @pytest.mark.parametrize("key", ["c/0", "foo/c/0.0", "foo/0/0"]) @pytest.mark.parametrize(