Skip to content
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

fix file modes #2000

Merged
merged 27 commits into from
Jul 26, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
32e84eb
fix file modes
brokkoli71 Jun 27, 2024
6d66f2a
change store mode from literal to class of properties
brokkoli71 Jul 1, 2024
ab353c9
raise FileNotFoundError instead of KeyError
brokkoli71 Jul 1, 2024
9d83b6e
Merge branch 'refs/heads/master' into fix-file-modes
brokkoli71 Jul 1, 2024
a964cb5
rename OpenMode to AccessMode
brokkoli71 Jul 3, 2024
1ff5861
rename AccessMode parameters
brokkoli71 Jul 3, 2024
2340e8b
enforce AccessMode for MemoryStore and RemoteStore
brokkoli71 Jul 3, 2024
cab827c
fix RemoteStore
brokkoli71 Jul 3, 2024
b9f49b4
fix RemoteStore
brokkoli71 Jul 3, 2024
8b37537
Merge remote-tracking branch 'origin/fix-file-modes' into fix-file-modes
brokkoli71 Jul 3, 2024
1d46753
formatting
brokkoli71 Jul 3, 2024
5f876d2
fix RemoteStore._exists
brokkoli71 Jul 3, 2024
218a527
Revert "fix RemoteStore._exists"
brokkoli71 Jul 3, 2024
edd3630
create async Store.open()
brokkoli71 Jul 4, 2024
01e3fa2
make Store.open() classmethod
brokkoli71 Jul 4, 2024
f016d34
async clear and root_exists in Store
brokkoli71 Jul 4, 2024
6dbf994
fix test_remote.py:test_basic
brokkoli71 Jul 4, 2024
2c372aa
fix RemoteStore.open
brokkoli71 Jul 5, 2024
f921f9e
Merge branch 'refs/heads/master' into fix-file-modes
brokkoli71 Jul 5, 2024
f03e4da
remove unnecessary import zarr in tests
brokkoli71 Jul 18, 2024
0c12510
rename root_exists to (not) empty, test and fix store.empty, store.clear
brokkoli71 Jul 18, 2024
7932593
mypy
brokkoli71 Jul 18, 2024
ef0aef8
Merge branch 'v3' into fix-file-modes
brokkoli71 Jul 18, 2024
830dd7e
incorporate feedback on store._open()
brokkoli71 Jul 20, 2024
3abfad6
rename store.ensure_open to store._ensure_open
brokkoli71 Jul 20, 2024
7d75edc
Merge branch 'refs/heads/master' into fix-file-modes
brokkoli71 Jul 26, 2024
6616185
Merge branch 'refs/heads/master' into fix-file-modes
brokkoli71 Jul 26, 2024
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
37 changes: 25 additions & 12 deletions src/zarr/abc/store.py
Original file line number Diff line number Diff line change
@@ -1,31 +1,44 @@
from abc import ABC, abstractmethod
from collections.abc import AsyncGenerator
from typing import Protocol, runtime_checkable
from typing import NamedTuple, Protocol, runtime_checkable

from typing_extensions import Self

from zarr.buffer import Buffer, BufferPrototype
from zarr.common import BytesLike, OpenMode
from zarr.common import BytesLike, OpenModeLiteral


class OpenMode(NamedTuple):
overwrite: bool
can_create: bool
can_open_existing: bool
is_writable: bool

@classmethod
def from_str(cls, mode: str) -> Self:
if mode not in ("r", "r+", "a", "w", "w-"):
raise ValueError("mode must be one of 'r', 'r+', 'w', 'w-', 'a'")
return cls(
overwrite=mode == "w",
can_create=mode in ("a", "w", "w-"),
can_open_existing=mode in ("r", "r+", "a"),
is_writable=mode in ("r+", "a", "w", "w-"),
)

brokkoli71 marked this conversation as resolved.
Show resolved Hide resolved

class Store(ABC):
_mode: OpenMode

def __init__(self, mode: OpenMode = "r"):
if mode not in ("r", "r+", "w", "w-", "a"):
raise ValueError("mode must be one of 'r', 'r+', 'w', 'w-', 'a'")
self._mode = mode
def __init__(self, mode: OpenModeLiteral = "r"):
self._mode = OpenMode.from_str(mode)

@property
def mode(self) -> OpenMode:
"""Access mode of the store."""
return self._mode

@property
def writeable(self) -> bool:
"""Is the store writeable?"""
return self.mode in ("a", "w", "w-")

def _check_writable(self) -> None:
if not self.writeable:
if not self.mode.is_writable:
raise ValueError("store mode does not support writing")

@abstractmethod
Expand Down
30 changes: 16 additions & 14 deletions src/zarr/api/asynchronous.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
from zarr.array import Array, AsyncArray
from zarr.buffer import NDArrayLike
from zarr.chunk_key_encodings import ChunkKeyEncoding
from zarr.common import JSON, ChunkCoords, MemoryOrder, OpenMode, ZarrFormat
from zarr.common import JSON, ChunkCoords, MemoryOrder, OpenModeLiteral, ZarrFormat
from zarr.group import AsyncGroup
from zarr.metadata import ArrayV2Metadata, ArrayV3Metadata
from zarr.store import (
Expand Down Expand Up @@ -158,7 +158,7 @@ async def load(
async def open(
*,
store: StoreLike | None = None,
mode: OpenMode | None = None, # type and value changed
mode: OpenModeLiteral | None = None, # type and value changed
zarr_version: ZarrFormat | None = None, # deprecated
zarr_format: ZarrFormat | None = None,
path: str | None = None,
Expand Down Expand Up @@ -195,9 +195,9 @@ async def open(
store_path = store_path / path

try:
return await open_array(store=store_path, zarr_format=zarr_format, **kwargs)
return await open_array(store=store_path, zarr_format=zarr_format, mode=mode, **kwargs)
except KeyError:
return await open_group(store=store_path, zarr_format=zarr_format, **kwargs)
return await open_group(store=store_path, zarr_format=zarr_format, mode=mode, **kwargs)


async def open_consolidated(*args: Any, **kwargs: Any) -> AsyncGroup:
Expand Down Expand Up @@ -451,7 +451,7 @@ async def group(
async def open_group(
*, # Note: this is a change from v2
store: StoreLike | None = None,
mode: OpenMode | None = None, # not used
mode: OpenModeLiteral | None = None, # not used
cache_attrs: bool | None = None, # not used, default changed
synchronizer: Any = None, # not used
path: str | None = None,
Expand Down Expand Up @@ -682,7 +682,7 @@ async def create(
if meta_array is not None:
warnings.warn("meta_array is not yet implemented", RuntimeWarning, stacklevel=2)

mode = cast(OpenMode, "r" if read_only else "w")
mode = kwargs.pop("mode", cast(OpenModeLiteral, "r" if read_only else "w"))
store_path = make_store_path(store, mode=mode)
if path is not None:
store_path = store_path / path
Expand Down Expand Up @@ -862,14 +862,16 @@ async def open_array(

try:
return await AsyncArray.open(store_path, zarr_format=zarr_format)
except KeyError as e:
if store_path.store.writeable:
pass
else:
raise e

# if array was not found, create it
return await create(store=store, path=path, zarr_format=zarr_format, **kwargs)
except FileNotFoundError as e:
if store_path.store.mode.can_create:
return await create(
store=store_path,
path=path,
zarr_format=zarr_format,
overwrite=store_path.store.mode.overwrite,
**kwargs,
)
raise e


async def open_like(a: ArrayLike, path: str, **kwargs: Any) -> AsyncArray:
Expand Down
6 changes: 3 additions & 3 deletions src/zarr/api/synchronous.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
import zarr.api.asynchronous as async_api
from zarr.array import Array, AsyncArray
from zarr.buffer import NDArrayLike
from zarr.common import JSON, ChunkCoords, OpenMode, ZarrFormat
from zarr.common import JSON, ChunkCoords, OpenModeLiteral, ZarrFormat
from zarr.group import Group
from zarr.store import StoreLike
from zarr.sync import sync
Expand Down Expand Up @@ -36,7 +36,7 @@ def load(
def open(
*,
store: StoreLike | None = None,
mode: OpenMode | None = None, # type and value changed
mode: OpenModeLiteral | None = None, # type and value changed
zarr_version: ZarrFormat | None = None, # deprecated
zarr_format: ZarrFormat | None = None,
path: str | None = None,
Expand Down Expand Up @@ -161,7 +161,7 @@ def group(
def open_group(
*, # Note: this is a change from v2
store: StoreLike | None = None,
mode: OpenMode | None = None, # not used in async api
mode: OpenModeLiteral | None = None, # not used in async api
cache_attrs: bool | None = None, # default changed, not used in async api
synchronizer: Any = None, # not used in async api
path: str | None = None,
Expand Down
8 changes: 4 additions & 4 deletions src/zarr/array.py
Original file line number Diff line number Diff line change
Expand Up @@ -332,11 +332,11 @@ async def open(
(store_path / ZARRAY_JSON).get(), (store_path / ZATTRS_JSON).get()
)
if zarray_bytes is None:
raise KeyError(store_path) # filenotfounderror?
raise FileNotFoundError(store_path)
elif zarr_format == 3:
zarr_json_bytes = await (store_path / ZARR_JSON).get()
if zarr_json_bytes is None:
raise KeyError(store_path) # filenotfounderror?
raise FileNotFoundError(store_path)
elif zarr_format is None:
zarr_json_bytes, zarray_bytes, zattrs_bytes = await gather(
(store_path / ZARR_JSON).get(),
Expand All @@ -348,7 +348,7 @@ async def open(
# alternatively, we could warn and favor v3
raise ValueError("Both zarr.json and .zarray objects exist")
if zarr_json_bytes is None and zarray_bytes is None:
raise KeyError(store_path) # filenotfounderror?
raise FileNotFoundError(store_path)
# set zarr_format based on which keys were found
if zarr_json_bytes is not None:
zarr_format = 3
Expand Down Expand Up @@ -403,7 +403,7 @@ def attrs(self) -> dict[str, JSON]:

@property
def read_only(self) -> bool:
return bool(not self.store_path.store.writeable)
return not bool(self.store_path.store.mode.is_writable)

@property
def path(self) -> str:
Expand Down
2 changes: 1 addition & 1 deletion src/zarr/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
ZarrFormat = Literal[2, 3]
JSON = None | str | int | float | Enum | dict[str, "JSON"] | list["JSON"] | tuple["JSON", ...]
MemoryOrder = Literal["C", "F"]
OpenMode = Literal["r", "r+", "a", "w", "w-"]
OpenModeLiteral = Literal["r", "r+", "a", "w", "w-"]


def product(tup: ChunkCoords) -> int:
Expand Down
2 changes: 1 addition & 1 deletion src/zarr/group.py
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ async def open(
# alternatively, we could warn and favor v3
raise ValueError("Both zarr.json and .zgroup objects exist")
if zarr_json_bytes is None and zgroup_bytes is None:
raise KeyError(store_path) # filenotfounderror?
raise FileNotFoundError(store_path)
# set zarr_format based on which keys were found
if zarr_json_bytes is not None:
zarr_format = 3
Expand Down
12 changes: 7 additions & 5 deletions src/zarr/store/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@
from pathlib import Path
from typing import Any

from zarr.abc.store import Store
from zarr.abc.store import OpenMode, Store
from zarr.buffer import Buffer, BufferPrototype, default_buffer_prototype
from zarr.common import OpenMode
from zarr.common import OpenModeLiteral
from zarr.store.local import LocalStore
from zarr.store.memory import MemoryStore

Expand Down Expand Up @@ -66,14 +66,16 @@ def __eq__(self, other: Any) -> bool:
StoreLike = Store | StorePath | Path | str


def make_store_path(store_like: StoreLike | None, *, mode: OpenMode | None = None) -> StorePath:
def make_store_path(
store_like: StoreLike | None, *, mode: OpenModeLiteral | None = None
) -> StorePath:
if isinstance(store_like, StorePath):
if mode is not None:
assert mode == store_like.store.mode
assert OpenMode.from_str(mode) == store_like.store.mode
return store_like
elif isinstance(store_like, Store):
if mode is not None:
assert mode == store_like.mode
assert OpenMode.from_str(mode) == store_like.mode
return StorePath(store_like)
elif store_like is None:
if mode is None:
Expand Down
13 changes: 10 additions & 3 deletions src/zarr/store/local.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

from zarr.abc.store import Store
from zarr.buffer import Buffer, BufferPrototype
from zarr.common import OpenMode, concurrent_map, to_thread
from zarr.common import OpenModeLiteral, concurrent_map, to_thread


def _get(
Expand Down Expand Up @@ -71,14 +71,21 @@ class LocalStore(Store):

root: Path

def __init__(self, root: Path | str, *, mode: OpenMode = "r"):
def __init__(self, root: Path | str, *, mode: OpenModeLiteral = "r"):
super().__init__(mode=mode)
if isinstance(root, str):
root = Path(root)
assert isinstance(root, Path)

self.root = root

if root.exists():
if self.mode.can_open_existing:
pass
elif self.mode.overwrite:
shutil.rmtree(root)
else:
raise FileExistsError(f"LocalStore: {root} already exists")

def __str__(self) -> str:
return f"file://{self.root}"

Expand Down
4 changes: 2 additions & 2 deletions src/zarr/store/memory.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

from zarr.abc.store import Store
from zarr.buffer import Buffer, BufferPrototype
from zarr.common import OpenMode, concurrent_map
from zarr.common import OpenModeLiteral, concurrent_map
from zarr.store.utils import _normalize_interval_index


Expand All @@ -18,7 +18,7 @@ class MemoryStore(Store):
_store_dict: MutableMapping[str, Buffer]

def __init__(
self, store_dict: MutableMapping[str, Buffer] | None = None, *, mode: OpenMode = "r"
self, store_dict: MutableMapping[str, Buffer] | None = None, *, mode: OpenModeLiteral = "r"
):
super().__init__(mode=mode)
self._store_dict = store_dict or {}
Expand Down
6 changes: 3 additions & 3 deletions src/zarr/store/remote.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@
import fsspec

from zarr.abc.store import Store
from zarr.buffer import Buffer, BufferPrototype
from zarr.common import OpenMode
from zarr.buffer import BufferPrototype
from zarr.common import OpenModeLiteral
from zarr.store.core import _dereference_path

if TYPE_CHECKING:
Expand All @@ -32,7 +32,7 @@ class RemoteStore(Store):
def __init__(
self,
url: UPath | str,
mode: OpenMode = "r",
mode: OpenModeLiteral = "r",
allowed_exceptions: tuple[type[Exception], ...] = (
FileNotFoundError,
IsADirectoryError,
Expand Down
19 changes: 7 additions & 12 deletions src/zarr/testing/store.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

import pytest

from zarr.abc.store import Store
from zarr.abc.store import OpenMode, Store
from zarr.buffer import Buffer, default_buffer_prototype
from zarr.store.utils import _normalize_interval_index
from zarr.testing.utils import assert_bytes_equal
Expand Down Expand Up @@ -43,22 +43,17 @@ def test_store_type(self, store: S) -> None:
assert isinstance(store, self.store_cls)

def test_store_mode(self, store: S, store_kwargs: dict[str, Any]) -> None:
assert store.mode == "w", store.mode
assert store.writeable
assert store.mode == OpenMode.from_str("w")
assert store.mode.is_writable

with pytest.raises(AttributeError):
store.mode = "w" # type: ignore[misc]

# read-only
kwargs = {**store_kwargs, "mode": "r"}
read_store = self.store_cls(**kwargs)
assert read_store.mode == "r", read_store.mode
assert not read_store.writeable
store.mode = OpenMode.from_str("w") # type: ignore[misc]

async def test_not_writable_store_raises(self, store_kwargs: dict[str, Any]) -> None:
kwargs = {**store_kwargs, "mode": "r"}
store = self.store_cls(**kwargs)
assert not store.writeable
assert store.mode == OpenMode.from_str("r")
assert not store.mode.is_writable

# set
with pytest.raises(ValueError):
Expand Down Expand Up @@ -102,7 +97,7 @@ async def test_set(self, store: S, key: str, data: bytes) -> None:
"""
Ensure that data can be written to the store using the store.set method.
"""
assert store.writeable
assert store.mode.is_writable
data_buf = Buffer.from_bytes(data)
await store.set(key, data_buf)
observed = self.get(store, key)
Expand Down
Loading