Skip to content

Commit 35d709a

Browse files
authored
Use Error subclasses instead of functions. (#590)
I believe this has several advantage: 1) reading the code it is obvious we are directly raising instead of doing more stuff. 2) in traceback the last frame will be correct, so it's easier to find where the error occurs. 3) All of these are now subclasses which can be caught independently by libraries while still being subclass of ValueError, so backward compatible. Can be caught without catching other ValueErrors. 4) the name of the class is descriptive I'm not too sure about some naming. Is ContainsXxxxError sufficient to understand that it contain Xxxxx but Xxxx was not expected ?
1 parent add56dd commit 35d709a

File tree

7 files changed

+99
-51
lines changed

7 files changed

+99
-51
lines changed

docs/release.rst

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,10 @@ Next release
1717
* Explicitly close stores during testing.
1818
By :user:`Elliott Sales de Andrade <QuLogic>`; :issue:`442`
1919

20+
* Many of the convenience functions to emit errors (``err_*`` from
21+
``zarr.errors`` have been replaced by ``ValueError`` subclasses. The
22+
functions are deprecated and will be removed in the future. :issue:`590` )
23+
2024
* Improve consistency of terminology regarding arrays and datasets in the
2125
documentation.
2226
By :user:`Josh Moore <joshmoore>`; :issue:`571`.

zarr/convenience.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
from zarr.core import Array
99
from zarr.creation import array as _create_array
1010
from zarr.creation import normalize_store_arg, open_array
11-
from zarr.errors import CopyError, err_path_not_found
11+
from zarr.errors import CopyError, PathNotFoundError
1212
from zarr.hierarchy import Group
1313
from zarr.hierarchy import group as _create_group
1414
from zarr.hierarchy import open_group
@@ -99,7 +99,7 @@ def open(store=None, mode='a', **kwargs):
9999
elif contains_group(store, path):
100100
return open_group(store, mode=mode, **kwargs)
101101
else:
102-
err_path_not_found(path)
102+
raise PathNotFoundError(path)
103103

104104

105105
def save_array(store, arr, **kwargs):

zarr/core.py

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212

1313
from zarr.attrs import Attributes
1414
from zarr.codecs import AsType, get_codec
15-
from zarr.errors import err_array_not_found, err_read_only
15+
from zarr.errors import ArrayNotFoundError, ReadOnlyError
1616
from zarr.indexing import (BasicIndexer, CoordinateIndexer, MaskIndexer,
1717
OIndex, OrthogonalIndexer, VIndex, check_fields,
1818
check_no_multi_fields, ensure_tuple,
@@ -149,7 +149,7 @@ def _load_metadata_nosync(self):
149149
mkey = self._key_prefix + array_meta_key
150150
meta_bytes = self._store[mkey]
151151
except KeyError:
152-
err_array_not_found(self._path)
152+
raise ArrayNotFoundError(self._path)
153153
else:
154154

155155
# decode and store metadata as instance members
@@ -1197,7 +1197,7 @@ def set_basic_selection(self, selection, value, fields=None):
11971197

11981198
# guard conditions
11991199
if self._read_only:
1200-
err_read_only()
1200+
raise ReadOnlyError()
12011201

12021202
# refresh metadata
12031203
if not self._cache_metadata:
@@ -1288,7 +1288,7 @@ def set_orthogonal_selection(self, selection, value, fields=None):
12881288

12891289
# guard conditions
12901290
if self._read_only:
1291-
err_read_only()
1291+
raise ReadOnlyError()
12921292

12931293
# refresh metadata
12941294
if not self._cache_metadata:
@@ -1360,7 +1360,7 @@ def set_coordinate_selection(self, selection, value, fields=None):
13601360

13611361
# guard conditions
13621362
if self._read_only:
1363-
err_read_only()
1363+
raise ReadOnlyError()
13641364

13651365
# refresh metadata
13661366
if not self._cache_metadata:
@@ -1441,7 +1441,7 @@ def set_mask_selection(self, selection, value, fields=None):
14411441

14421442
# guard conditions
14431443
if self._read_only:
1444-
err_read_only()
1444+
raise ReadOnlyError()
14451445

14461446
# refresh metadata
14471447
if not self._cache_metadata:
@@ -1966,7 +1966,7 @@ def _write_op(self, f, *args, **kwargs):
19661966

19671967
# guard condition
19681968
if self._read_only:
1969-
err_read_only()
1969+
raise ReadOnlyError()
19701970

19711971
return self._synchronized_op(f, *args, **kwargs)
19721972

zarr/creation.py

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,11 @@
55
from numcodecs.registry import codec_registry
66

77
from zarr.core import Array
8-
from zarr.errors import (err_array_not_found, err_contains_array,
9-
err_contains_group)
8+
from zarr.errors import (
9+
ArrayNotFoundError,
10+
ContainsArrayError,
11+
ContainsGroupError,
12+
)
1013
from zarr.n5 import N5Store
1114
from zarr.storage import (DirectoryStore, ZipStore, contains_array,
1215
contains_group, default_compressor, init_array,
@@ -466,9 +469,9 @@ def open_array(store=None, mode='a', shape=None, chunks=True, dtype=None,
466469

467470
if mode in ['r', 'r+']:
468471
if contains_group(store, path=path):
469-
err_contains_group(path)
472+
raise ContainsGroupError(path)
470473
elif not contains_array(store, path=path):
471-
err_array_not_found(path)
474+
raise ArrayNotFoundError(path)
472475

473476
elif mode == 'w':
474477
init_array(store, shape=shape, chunks=chunks, dtype=dtype,
@@ -478,7 +481,7 @@ def open_array(store=None, mode='a', shape=None, chunks=True, dtype=None,
478481

479482
elif mode == 'a':
480483
if contains_group(store, path=path):
481-
err_contains_group(path)
484+
raise ContainsGroupError(path)
482485
elif not contains_array(store, path=path):
483486
init_array(store, shape=shape, chunks=chunks, dtype=dtype,
484487
compressor=compressor, fill_value=fill_value,
@@ -487,9 +490,9 @@ def open_array(store=None, mode='a', shape=None, chunks=True, dtype=None,
487490

488491
elif mode in ['w-', 'x']:
489492
if contains_group(store, path=path):
490-
err_contains_group(path)
493+
raise ContainsGroupError(path)
491494
elif contains_array(store, path=path):
492-
err_contains_array(path)
495+
raise ContainsArrayError(path)
493496
else:
494497
init_array(store, shape=shape, chunks=chunks, dtype=dtype,
495498
compressor=compressor, fill_value=fill_value,

zarr/errors.py

Lines changed: 40 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -9,37 +9,69 @@ class CopyError(RuntimeError):
99
pass
1010

1111

12+
class _BaseZarrError(ValueError):
13+
_msg = ""
14+
15+
def __init__(self, *args):
16+
super().__init__(self._msg.format(*args))
17+
18+
19+
class ContainsGroupError(_BaseZarrError):
20+
_msg = "path {0!r} contains a group"
21+
22+
1223
def err_contains_group(path):
13-
raise ValueError('path %r contains a group' % path)
24+
raise ContainsGroupError(path) # pragma: no cover
25+
26+
27+
class ContainsArrayError(_BaseZarrError):
28+
_msg = "path {0!r} contains an array"
1429

1530

1631
def err_contains_array(path):
17-
raise ValueError('path %r contains an array' % path)
32+
raise ContainsArrayError(path) # pragma: no cover
33+
34+
35+
class ArrayNotFoundError(_BaseZarrError):
36+
_msg = "array not found at path %r' {0!r}"
1837

1938

2039
def err_array_not_found(path):
21-
raise ValueError('array not found at path %r' % path)
40+
raise ArrayNotFoundError(path) # pragma: no cover
41+
42+
43+
class GroupNotFoundError(_BaseZarrError):
44+
_msg = "group not found at path {0!r}"
2245

2346

2447
def err_group_not_found(path):
25-
raise ValueError('group not found at path %r' % path)
48+
raise GroupNotFoundError(path) # pragma: no cover
49+
50+
51+
class PathNotFoundError(_BaseZarrError):
52+
_msg = "nothing found at path {0!r}"
2653

2754

2855
def err_path_not_found(path):
29-
raise ValueError('nothing found at path %r' % path)
56+
raise PathNotFoundError(path) # pragma: no cover
3057

3158

3259
def err_bad_compressor(compressor):
3360
raise ValueError('bad compressor; expected Codec object, found %r' %
3461
compressor)
3562

3663

37-
def err_fspath_exists_notdir(fspath):
38-
raise ValueError('path exists but is not a directory: %r' % fspath)
64+
class FSPathExistNotDir(GroupNotFoundError):
65+
_msg = "path exists but is not a directory: %r"
66+
67+
68+
class ReadOnlyError(PermissionError):
69+
def __init__(self):
70+
super().__init__("object is read-only")
3971

4072

4173
def err_read_only():
42-
raise PermissionError('object is read-only')
74+
raise ReadOnlyError() # pragma: no cover
4375

4476

4577
def err_boundscheck(dim_len):

zarr/hierarchy.py

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,12 @@
99
from zarr.creation import (array, create, empty, empty_like, full, full_like,
1010
normalize_store_arg, ones, ones_like, zeros,
1111
zeros_like)
12-
from zarr.errors import (err_contains_array, err_contains_group,
13-
err_group_not_found, err_read_only)
12+
from zarr.errors import (
13+
ContainsArrayError,
14+
ContainsGroupError,
15+
GroupNotFoundError,
16+
ReadOnlyError,
17+
)
1418
from zarr.meta import decode_group_metadata
1519
from zarr.storage import (MemoryStore, attrs_key, contains_array,
1620
contains_group, group_meta_key, init_group, listdir,
@@ -105,14 +109,14 @@ def __init__(self, store, path=None, read_only=False, chunk_store=None,
105109

106110
# guard conditions
107111
if contains_array(store, path=self._path):
108-
err_contains_array(path)
112+
raise ContainsArrayError(path)
109113

110114
# initialize metadata
111115
try:
112116
mkey = self._key_prefix + group_meta_key
113117
meta_bytes = store[mkey]
114118
except KeyError:
115-
err_group_not_found(path)
119+
raise GroupNotFoundError(path)
116120
else:
117121
meta = decode_group_metadata(meta_bytes)
118122
self._meta = meta
@@ -645,7 +649,7 @@ def _write_op(self, f, *args, **kwargs):
645649

646650
# guard condition
647651
if self._read_only:
648-
err_read_only()
652+
raise ReadOnlyError()
649653

650654
if self._synchronizer is None:
651655
# no synchronization
@@ -1154,24 +1158,24 @@ def open_group(store=None, mode='a', cache_attrs=True, synchronizer=None, path=N
11541158

11551159
if mode in ['r', 'r+']:
11561160
if contains_array(store, path=path):
1157-
err_contains_array(path)
1161+
raise ContainsArrayError(path)
11581162
elif not contains_group(store, path=path):
1159-
err_group_not_found(path)
1163+
raise GroupNotFoundError(path)
11601164

11611165
elif mode == 'w':
11621166
init_group(store, overwrite=True, path=path, chunk_store=chunk_store)
11631167

11641168
elif mode == 'a':
11651169
if contains_array(store, path=path):
1166-
err_contains_array(path)
1170+
raise ContainsArrayError(path)
11671171
if not contains_group(store, path=path):
11681172
init_group(store, path=path, chunk_store=chunk_store)
11691173

11701174
elif mode in ['w-', 'x']:
11711175
if contains_array(store, path=path):
1172-
err_contains_array(path)
1176+
raise ContainsArrayError(path)
11731177
elif contains_group(store, path=path):
1174-
err_contains_group(path)
1178+
raise ContainsGroupError(path)
11751179
else:
11761180
init_group(store, path=path, chunk_store=chunk_store)
11771181

0 commit comments

Comments
 (0)