From c8297ce08dec29cc3b1cd23c22ad7ae212b81d57 Mon Sep 17 00:00:00 2001 From: David Stansby Date: Tue, 29 Jul 2025 20:11:10 +0100 Subject: [PATCH 1/3] Prevent mode='r+' from creating new directories --- src/zarr/storage/_common.py | 2 +- src/zarr/storage/_local.py | 42 +++++++++++++++++++++++++++++++++---- 2 files changed, 39 insertions(+), 5 deletions(-) diff --git a/src/zarr/storage/_common.py b/src/zarr/storage/_common.py index 3a63b30e9b..66a53c19df 100644 --- a/src/zarr/storage/_common.py +++ b/src/zarr/storage/_common.py @@ -358,7 +358,7 @@ async def make_store_path( elif isinstance(store_like, Path): # Create a new LocalStore - store = await LocalStore.open(root=store_like, read_only=_read_only) + store = await LocalStore.open(root=store_like, mode=mode) elif isinstance(store_like, str): # Either a FSSpec URI or a local filesystem path diff --git a/src/zarr/storage/_local.py b/src/zarr/storage/_local.py index 43e585415d..32a9d1a24b 100644 --- a/src/zarr/storage/_local.py +++ b/src/zarr/storage/_local.py @@ -5,7 +5,7 @@ import os import shutil from pathlib import Path -from typing import TYPE_CHECKING +from typing import TYPE_CHECKING, Self from zarr.abc.store import ( ByteRequest, @@ -16,7 +16,7 @@ ) from zarr.core.buffer import Buffer from zarr.core.buffer.core import default_buffer_prototype -from zarr.core.common import concurrent_map +from zarr.core.common import AccessModeLiteral, concurrent_map if TYPE_CHECKING: from collections.abc import AsyncIterator, Iterable @@ -102,16 +102,50 @@ def __init__(self, root: Path | str, *, read_only: bool = False) -> None: ) self.root = root - def with_read_only(self, read_only: bool = False) -> LocalStore: + def with_read_only(self, read_only: bool = False) -> Self: # docstring inherited return type(self)( root=self.root, read_only=read_only, ) - async def _open(self) -> None: + @classmethod + async def open( + cls, root: Path | str, *, read_only: bool = False, mode: AccessModeLiteral | None = None + ) -> Self: + """ + Create and open the store. + + Parameters + ---------- + root : str or Path + Directory to use as root of store. + read_only : bool + Whether the store is read-only + mode : + Mode in which to create the store. This only affects opening the store, + and the final read-only state of the store is controlled through the + read_only parameter. + + Returns + ------- + Store + The opened store instance. + """ + if mode is not None: + read_only_creation = mode in ["r", "r+"] + else: + read_only_creation = read_only + store = cls(root, read_only=read_only_creation) + await store._open() + return store.with_read_only(read_only) + + async def _open(self, *, mode: AccessModeLiteral | None = None) -> None: if not self.read_only: self.root.mkdir(parents=True, exist_ok=True) + + if not self.root.exists(): + raise FileNotFoundError(f"{self.root} does not exist") return await super()._open() async def clear(self) -> None: From 2b2b3874292841e9eb02124ca88f591535c3be61 Mon Sep 17 00:00:00 2001 From: David Stansby Date: Tue, 29 Jul 2025 20:11:27 +0100 Subject: [PATCH 2/3] Add test --- tests/test_api.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/test_api.py b/tests/test_api.py index 01fb40f050..2d6b19e7c9 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -285,8 +285,12 @@ def test_open_with_mode_r(tmp_path: Path) -> None: def test_open_with_mode_r_plus(tmp_path: Path) -> None: # 'r+' means read/write (must exist) + new_store_path = tmp_path / "new_store.zarr" + assert not new_store_path.exists(), "Test should operate on non-existent directory" with pytest.raises(FileNotFoundError): - zarr.open(store=tmp_path, mode="r+") + zarr.open(store=new_store_path, mode="r+") + assert not new_store_path.exists(), "mode='r+' should not create directory" + zarr.ones(store=tmp_path, shape=(3, 3)) z2 = zarr.open(store=tmp_path, mode="r+") assert isinstance(z2, Array) From 9342439373fdd7805b943d82dd839365a6b40b5e Mon Sep 17 00:00:00 2001 From: David Stansby Date: Mon, 4 Aug 2025 11:14:32 +0100 Subject: [PATCH 3/3] Open store for mode = r+ --- src/zarr/storage/_local.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/zarr/storage/_local.py b/src/zarr/storage/_local.py index 32a9d1a24b..1229ec316a 100644 --- a/src/zarr/storage/_local.py +++ b/src/zarr/storage/_local.py @@ -132,13 +132,19 @@ async def open( Store The opened store instance. """ + # If mode = 'r+', want to open in read only mode (fail if exists), + # but return a writeable store if mode is not None: read_only_creation = mode in ["r", "r+"] else: read_only_creation = read_only store = cls(root, read_only=read_only_creation) await store._open() - return store.with_read_only(read_only) + + # Set read_only state + store = store.with_read_only(read_only) + await store._open() + return store async def _open(self, *, mode: AccessModeLiteral | None = None) -> None: if not self.read_only: