Skip to content

Commit 432d975

Browse files
committed
Merge branch 'main' of github.com:zarr-developers/zarr-python into docs/dtype-docs
2 parents ae268b9 + 5731c6c commit 432d975

File tree

16 files changed

+113
-49
lines changed

16 files changed

+113
-49
lines changed

.pre-commit-config.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ repos:
2222
- id: check-yaml
2323
- id: trailing-whitespace
2424
- repo: https://github.com/pre-commit/mirrors-mypy
25-
rev: v1.15.0
25+
rev: v1.16.1
2626
hooks:
2727
- id: mypy
2828
files: src|tests

changes/3068.bugfix.rst

Lines changed: 0 additions & 1 deletion
This file was deleted.

changes/3156.bugfix.rst

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Trying to open a StorePath/Array with ``mode='r'`` when the store is not read-only creates a read-only copy of the store.

codecov.yml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,12 @@ coverage:
33
patch:
44
default:
55
target: auto
6+
after_n_builds: 10 # Wait for all 10 reports before updating the status
67
project:
78
default:
89
target: auto
910
threshold: 0.1
11+
after_n_builds: 2 # Wait for all 10 reports before updating the status
1012
comment: false
13+
github_checks:
14+
annotations: false

src/zarr/core/buffer/core.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ def ravel(self, order: Literal["K", "A", "C", "F"] = ...) -> Self: ...
9393

9494
def all(self) -> bool: ...
9595

96-
def __eq__(self, other: object) -> Self: # type: ignore[explicit-override, override]
96+
def __eq__(self, other: object) -> Self: # type: ignore[override]
9797
"""Element-wise equal
9898
9999
Notes

src/zarr/core/common.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
from typing import (
1111
TYPE_CHECKING,
1212
Any,
13+
Final,
1314
Generic,
1415
Literal,
1516
TypedDict,
@@ -39,6 +40,7 @@
3940
JSON = str | int | float | Mapping[str, "JSON"] | Sequence["JSON"] | None
4041
MemoryOrder = Literal["C", "F"]
4142
AccessModeLiteral = Literal["r", "r+", "a", "w", "w-"]
43+
ANY_ACCESS_MODE: Final = "r", "r+", "a", "w", "w-"
4244
DimensionNames = Iterable[str | None] | None
4345

4446
TName = TypeVar("TName", bound=str)

src/zarr/core/dtype/npy/bytes.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -518,7 +518,7 @@ def _check_native_dtype(
518518
Bool
519519
True if the dtype is an instance of np.dtypes.VoidDType with no fields, False otherwise.
520520
"""
521-
return cls.dtype_cls is type(dtype) and dtype.fields is None # type: ignore[has-type]
521+
return cls.dtype_cls is type(dtype) and dtype.fields is None
522522

523523
@classmethod
524524
def from_native_dtype(cls, dtype: TBaseDType) -> Self:
@@ -549,7 +549,7 @@ def from_native_dtype(cls, dtype: TBaseDType) -> Self:
549549
if cls._check_native_dtype(dtype):
550550
return cls(length=dtype.itemsize)
551551
raise DataTypeValidationError(
552-
f"Invalid data type: {dtype}. Expected an instance of {cls.dtype_cls}" # type: ignore[has-type]
552+
f"Invalid data type: {dtype}. Expected an instance of {cls.dtype_cls}"
553553
)
554554

555555
def to_native_dtype(self) -> np.dtypes.VoidDType[int]:

src/zarr/core/dtype/npy/structured.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ def _check_native_dtype(cls, dtype: TBaseDType) -> TypeGuard[np.dtypes.VoidDType
6464
TypeGuard[np.dtypes.VoidDType]
6565
True if the dtype matches, False otherwise.
6666
"""
67-
return isinstance(dtype, cls.dtype_cls) and dtype.fields is not None # type: ignore[has-type]
67+
return isinstance(dtype, cls.dtype_cls) and dtype.fields is not None
6868

6969
@classmethod
7070
def from_native_dtype(cls, dtype: TBaseDType) -> Self:
@@ -104,7 +104,7 @@ def from_native_dtype(cls, dtype: TBaseDType) -> Self:
104104

105105
return cls(fields=tuple(fields))
106106
raise DataTypeValidationError(
107-
f"Invalid data type: {dtype}. Expected an instance of {cls.dtype_cls}" # type: ignore[has-type]
107+
f"Invalid data type: {dtype}. Expected an instance of {cls.dtype_cls}"
108108
)
109109

110110
def to_native_dtype(self) -> np.dtypes.VoidDType[int]:

src/zarr/core/dtype/npy/time.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -392,7 +392,7 @@ def _from_json_v2(cls, data: DTypeJSON) -> Self:
392392
return cls.from_native_dtype(np.dtype(name))
393393
msg = (
394394
f"Invalid JSON representation of {cls.__name__}. Got {data!r}, expected a string "
395-
f"representation of an instance of {cls.dtype_cls}" # type: ignore[has-type]
395+
f"representation of an instance of {cls.dtype_cls}"
396396
)
397397
raise DataTypeValidationError(msg)
398398

@@ -634,7 +634,7 @@ def _from_json_v2(cls, data: DTypeJSON) -> Self:
634634
return cls.from_native_dtype(np.dtype(name))
635635
msg = (
636636
f"Invalid JSON representation of {cls.__name__}. Got {data!r}, expected a string "
637-
f"representation of an instance of {cls.dtype_cls}" # type: ignore[has-type]
637+
f"representation of an instance of {cls.dtype_cls}"
638638
)
639639
raise DataTypeValidationError(msg)
640640

src/zarr/storage/_common.py

Lines changed: 55 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,14 @@
77

88
from zarr.abc.store import ByteRequest, Store
99
from zarr.core.buffer import Buffer, default_buffer_prototype
10-
from zarr.core.common import ZARR_JSON, ZARRAY_JSON, ZGROUP_JSON, AccessModeLiteral, ZarrFormat
10+
from zarr.core.common import (
11+
ANY_ACCESS_MODE,
12+
ZARR_JSON,
13+
ZARRAY_JSON,
14+
ZGROUP_JSON,
15+
AccessModeLiteral,
16+
ZarrFormat,
17+
)
1118
from zarr.errors import ContainsArrayAndGroupError, ContainsArrayError, ContainsGroupError
1219
from zarr.storage._local import LocalStore
1320
from zarr.storage._memory import MemoryStore
@@ -54,59 +61,82 @@ def __init__(self, store: Store, path: str = "") -> None:
5461
def read_only(self) -> bool:
5562
return self.store.read_only
5663

64+
@classmethod
65+
async def _create_open_instance(cls, store: Store, path: str) -> Self:
66+
"""Helper to create and return a StorePath instance."""
67+
await store._ensure_open()
68+
return cls(store, path)
69+
5770
@classmethod
5871
async def open(cls, store: Store, path: str, mode: AccessModeLiteral | None = None) -> Self:
5972
"""
6073
Open StorePath based on the provided mode.
6174
62-
* If the mode is 'w-' and the StorePath contains keys, raise a FileExistsError.
63-
* If the mode is 'w', delete all keys nested within the StorePath
64-
* If the mode is 'a', 'r', or 'r+', do nothing
75+
* If the mode is None, return an opened version of the store with no changes.
76+
* If the mode is 'r+', 'w-', 'w', or 'a' and the store is read-only, raise a ValueError.
77+
* If the mode is 'r' and the store is not read-only, return a copy of the store with read_only set to True.
78+
* If the mode is 'w-' and the store is not read-only and the StorePath contains keys, raise a FileExistsError.
79+
* If the mode is 'w' and the store is not read-only, delete all keys nested within the StorePath.
6580
6681
Parameters
6782
----------
6883
mode : AccessModeLiteral
6984
The mode to use when initializing the store path.
7085
86+
The accepted values are:
87+
88+
- ``'r'``: read only (must exist)
89+
- ``'r+'``: read/write (must exist)
90+
- ``'a'``: read/write (create if doesn't exist)
91+
- ``'w'``: read/write (overwrite if exists)
92+
- ``'w-'``: read/write (create if doesn't exist).
93+
7194
Raises
7295
------
7396
FileExistsError
7497
If the mode is 'w-' and the store path already exists.
7598
ValueError
7699
If the mode is not "r" and the store is read-only, or
77-
if the mode is "r" and the store is not read-only.
78100
"""
79101

80-
await store._ensure_open()
81-
self = cls(store, path)
82-
83102
# fastpath if mode is None
84103
if mode is None:
85-
return self
104+
return await cls._create_open_instance(store, path)
86105

87-
if store.read_only and mode != "r":
88-
raise ValueError(f"Store is read-only but mode is '{mode}'")
89-
if not store.read_only and mode == "r":
90-
raise ValueError(f"Store is not read-only but mode is '{mode}'")
106+
if mode not in ANY_ACCESS_MODE:
107+
raise ValueError(f"Invalid mode: {mode}, expected one of {ANY_ACCESS_MODE}")
91108

109+
if store.read_only:
110+
# Don't allow write operations on a read-only store
111+
if mode != "r":
112+
raise ValueError(
113+
f"Store is read-only but mode is {mode!r}. Create a writable store or use 'r' mode."
114+
)
115+
self = await cls._create_open_instance(store, path)
116+
elif mode == "r":
117+
# Create read-only copy for read mode on writable store
118+
try:
119+
read_only_store = store.with_read_only(True)
120+
except NotImplementedError as e:
121+
raise ValueError(
122+
"Store is not read-only but mode is 'r'. Unable to create a read-only copy of the store. "
123+
"Please use a read-only store or a storage class that implements .with_read_only()."
124+
) from e
125+
self = await cls._create_open_instance(read_only_store, path)
126+
else:
127+
# writable store and writable mode
128+
self = await cls._create_open_instance(store, path)
129+
130+
# Handle mode-specific operations
92131
match mode:
93132
case "w-":
94133
if not await self.is_empty():
95-
msg = (
96-
f"{self} is not empty, but `mode` is set to 'w-'."
97-
"Either remove the existing objects in storage,"
98-
"or set `mode` to a value that handles pre-existing objects"
99-
"in storage, like `a` or `w`."
134+
raise FileExistsError(
135+
f"Cannot create '{path}' with mode 'w-' because it already contains data. "
136+
f"Use mode 'w' to overwrite or 'a' to append."
100137
)
101-
raise FileExistsError(msg)
102138
case "w":
103139
await self.delete_dir()
104-
case "a" | "r" | "r+":
105-
# No init action
106-
pass
107-
case _:
108-
raise ValueError(f"Invalid mode: {mode}")
109-
110140
return self
111141

112142
async def get(

0 commit comments

Comments
 (0)