review errors

Author: alimanfoo

zarr/core.py

@@ -18,7 +18,7 @@
 from zarr.codecs import AsType, get_codec
 from zarr.indexing import (OIndex, OrthogonalIndexer, BasicIndexer, VIndex, CoordinateIndexer,
                            MaskIndexer, check_fields, pop_fields, ensure_tuple, is_scalar,
-                           is_contiguous_selection)
+                           is_contiguous_selection, err_too_many_indices, check_no_multi_fields)
 
 
 # noinspection PyUnresolvedReferences
@@ -173,7 +173,7 @@ def _refresh_metadata_nosync(self):
 
     def _flush_metadata_nosync(self):
         if self._is_view:
-            raise PermissionError('not permitted for views')
+            raise PermissionError('operation not permitted for views')
 
         if self._compressor:
             compressor_config = self._compressor.get_config()
@@ -403,6 +403,7 @@ def __len__(self):
         if self.shape:
             return self.shape[0]
         else:
+            # 0-dimensional array, same error message as numpy
             raise TypeError('len() of unsized object')
 
     def __getitem__(self, selection):
@@ -662,7 +663,7 @@ def _get_basic_selection_zd(self, selection, out=None, fields=None):
         # check selection is valid
         selection = ensure_tuple(selection)
         if selection not in ((), (Ellipsis,)):
-            raise IndexError('too many indices for array')
+            err_too_many_indices(selection, ())
 
         try:
             # obtain encoded data for chunk
@@ -1413,12 +1414,11 @@ def _set_basic_selection_zd(self, selection, value, fields=None):
         # check selection is valid
         selection = ensure_tuple(selection)
         if selection not in ((), (Ellipsis,)):
-            raise IndexError('too many indices for array')
+            err_too_many_indices(selection, self._shape)
 
         # check fields
         check_fields(fields, self._dtype)
-        if fields and isinstance(fields, list):
-            raise ValueError('multi-field assignment is not supported')
+        fields = check_no_multi_fields(fields)
 
         # obtain key for chunk
         ckey = self._chunk_key((0,))
@@ -1467,8 +1467,7 @@ def _set_selection(self, indexer, value, fields=None):
 
         # check fields are sensible
         check_fields(fields, self._dtype)
-        if fields and isinstance(fields, list):
-            raise ValueError('multi-field assignment is not supported')
+        fields = check_no_multi_fields(fields)
 
         # determine indices of chunks overlapping the selection
         sel_shape = indexer.shape
@@ -1944,7 +1943,8 @@ def _append_nosync(self, data, axis=0):
         data_shape_preserved = tuple(s for i, s in enumerate(data.shape)
                                      if i != axis)
         if self_shape_preserved != data_shape_preserved:
-            raise ValueError('shapes not compatible')
+            raise ValueError('shape of data to append is not compatible with the array; all '
+                             'dimensions must match except for the dimension being appended')
 
         # remember old shape
         old_shape = self._shape
@@ -2074,7 +2074,7 @@ def view(self, shape=None, chunks=None, dtype=None,
             ...     v.resize(20000)
             ... except PermissionError as e:
             ...     print(e)
-            not permitted for views
+            operation not permitted for views
 
         """
 

zarr/errors.py

@@ -50,3 +50,23 @@ def err_fspath_exists_notdir(fspath):
 
 def err_read_only():
     raise PermissionError('object is read-only')
+
+
+def err_boundscheck(dim_len):
+    raise IndexError('index out of bounds for dimension with length {}'
+                     .format(dim_len))
+
+
+def err_negative_step():
+    raise IndexError('only slices with step >= 1 are supported')
+
+
+def err_too_many_indices(selection, shape):
+    raise IndexError('too many indices for array; expected {}, got {}'
+                     .format(len(shape), len(selection)))
+
+
+def err_vindex_invalid_selection(selection):
+    raise IndexError('unsupported selection type for vectorized indexing; only coordinate '
+                     'selection (tuple of integer arrays) and mask selection (single '
+                     'Boolean array) are supported; got {!r}'.format(selection))

zarr/hierarchy.py

@@ -719,21 +719,27 @@ def _require_dataset_nosync(self, name, shape, dtype=None, exact=False,
         path = self._item_path(name)
 
         if contains_array(self._store, path):
+
+            # array already exists at path, validate that it is the right shape and type
+
             synchronizer = kwargs.get('synchronizer', self._synchronizer)
             cache_metadata = kwargs.get('cache_metadata', True)
             a = Array(self._store, path=path, read_only=self._read_only,
                       chunk_store=self._chunk_store, synchronizer=synchronizer,
                       cache_metadata=cache_metadata)
             shape = normalize_shape(shape)
             if shape != a.shape:
-                raise TypeError('shapes do not match')
+                raise TypeError('shape do not match existing array; expected {}, got {}'
+                                .format(a.shape, shape))
             dtype = np.dtype(dtype)
             if exact:
                 if dtype != a.dtype:
-                    raise TypeError('dtypes do not match exactly')
+                    raise TypeError('dtypes do not match exactly; expected {}, got {}'
+                                    .format(a.dtype, dtype))
             else:
                 if not np.can_cast(dtype, a.dtype):
-                    raise TypeError('dtypes cannot be safely cast')
+                    raise TypeError('dtypes ({}, {}) cannot be safely cast'
+                                    .format(dtype, a.dtype))
             return a
 
         else:

zarr/indexing.py

@@ -8,6 +8,10 @@
 import numpy as np
 
 
+from zarr.errors import (err_too_many_indices, err_boundscheck, err_negative_step,
+                         err_vindex_invalid_selection)
+
+
 def is_integer(x):
     return isinstance(x, numbers.Integral)
 
@@ -34,11 +38,6 @@ def is_scalar(value, dtype):
     return False
 
 
-def err_boundscheck(dim_len):
-    raise IndexError('index out of bounds for dimension with length {}'
-                     .format(dim_len))
-
-
 def normalize_integer_selection(dim_sel, dim_len):
 
     # normalize type to int
@@ -96,10 +95,6 @@ def ceildiv(a, b):
     return int(np.ceil(a / b))
 
 
-def err_negative_step():
-    raise IndexError('only slices with step >= 1 are supported')
-
-
 class SliceDimIndexer(object):
 
     def __init__(self, dim_sel, dim_len, dim_chunk_len):
@@ -160,6 +155,11 @@ def __iter__(self):
             yield ChunkDimProjection(dim_chunk_ix, dim_chunk_sel, dim_out_sel)
 
 
+def check_selection_length(selection, shape):
+    if len(selection) > len(shape):
+        err_too_many_indices(selection, shape)
+
+
 def replace_ellipsis(selection, shape):
 
     selection = ensure_tuple(selection)
@@ -193,9 +193,7 @@ def replace_ellipsis(selection, shape):
         selection += (slice(None),) * (len(shape) - len(selection))
 
     # check selection not too long
-    if len(selection) > len(shape):
-        raise IndexError('too many indices for array; expected {}, got {}'
-                         .format(len(shape), len(selection)))
+    check_selection_length(selection, shape)
 
     return selection
 
@@ -732,12 +730,6 @@ def __init__(self, selection, array):
         super(MaskIndexer, self).__init__(selection, array)
 
 
-def err_vindex_invalid_selection(selection):
-    raise IndexError('unsupported selection type for vectorized indexing; only coordinate '
-                     'selection (tuple of integer arrays) and mask selection (single '
-                     'Boolean array) are supported; got {!r}'.format(selection))
-
-
 class VIndex(object):
 
     def __init__(self, array):
@@ -792,6 +784,15 @@ def check_fields(fields, dtype):
         return dtype
 
 
+def check_no_multi_fields(fields):
+    if isinstance(fields, list):
+        if len(fields) == 1:
+            return fields[0]
+        elif len(fields) > 1:
+            raise IndexError('multiple fields are not supported for this operation')
+    return fields
+
+
 def pop_fields(selection):
     if isinstance(selection, str):
         # single field selection

zarr/tests/test_indexing.py

@@ -324,9 +324,9 @@ def test_set_basic_selection_0d():
     eq(v['bar'], z['bar'])
     eq(a['baz'], z['baz'])
     # multiple field assignment not supported
-    with assert_raises(ValueError):
+    with assert_raises(IndexError):
         z.set_basic_selection(Ellipsis, v[['foo', 'bar']], fields=['foo', 'bar'])
-    with assert_raises(ValueError):
+    with assert_raises(IndexError):
         z[..., 'foo', 'bar'] = v[['foo', 'bar']]
 
 
@@ -1213,6 +1213,7 @@ def test_set_selections_with_fields():
 
     fields_fixture = [
         'foo',
+        [],
         ['foo'],
         ['foo', 'bar'],
         ['foo', 'baz'],
@@ -1225,54 +1226,64 @@ def test_set_selections_with_fields():
     for fields in fields_fixture:
 
         # currently multi-field assignment is not supported in numpy, so we won't support it either
-        if isinstance(fields, list):
-            with assert_raises(ValueError):
-                z.set_basic_selection(Ellipsis, v[fields], fields=fields)
-            with assert_raises(ValueError):
-                z.set_orthogonal_selection([0, 2], v[fields], fields=fields)
-            with assert_raises(ValueError):
-                z.set_coordinate_selection([0, 2], v[fields], fields=fields)
-            with assert_raises(ValueError):
-                z.set_mask_selection([True, False, True], v[fields], fields=fields)
+        if isinstance(fields, list) and len(fields) > 1:
+            with assert_raises(IndexError):
+                z.set_basic_selection(Ellipsis, v, fields=fields)
+            with assert_raises(IndexError):
+                z.set_orthogonal_selection([0, 2], v, fields=fields)
+            with assert_raises(IndexError):
+                z.set_coordinate_selection([0, 2], v, fields=fields)
+            with assert_raises(IndexError):
+                z.set_mask_selection([True, False, True], v, fields=fields)
 
         else:
 
+            if isinstance(fields, list) and len(fields) == 1:
+                # work around numpy does not support multi-field assignment even if there is only
+                # one field
+                key = fields[0]
+            elif isinstance(fields, list) and len(fields) == 0:
+                # work around numpy ambiguity about what is a field selection
+                key = Ellipsis
+            else:
+                key = fields
+
             # setup expectation
             a[:] = ('', 0, 0)
             z[:] = ('', 0, 0)
             assert_array_equal(a, z[:])
-            a[fields] = v[fields]
+            a[key] = v[key]
             # total selection
-            z.set_basic_selection(Ellipsis, v[fields], fields=fields)
+            z.set_basic_selection(Ellipsis, v[key], fields=fields)
             assert_array_equal(a, z[:])
 
             # basic selection with slice
             a[:] = ('', 0, 0)
             z[:] = ('', 0, 0)
-            a[fields][0:2] = v[fields][0:2]
-            z.set_basic_selection(slice(0, 2), v[0:2][fields], fields=fields)
+            a[key][0:2] = v[key][0:2]
+            z.set_basic_selection(slice(0, 2), v[key][0:2], fields=fields)
             assert_array_equal(a, z[:])
 
             # orthogonal selection
             a[:] = ('', 0, 0)
             z[:] = ('', 0, 0)
             ix = [0, 2]
-            a[fields][ix] = v[fields][ix]
-            z.set_orthogonal_selection(ix, v[fields][ix], fields=fields)
+            a[key][ix] = v[key][ix]
+            z.set_orthogonal_selection(ix, v[key][ix], fields=fields)
             assert_array_equal(a, z[:])
 
             # coordinate selection
             a[:] = ('', 0, 0)
             z[:] = ('', 0, 0)
             ix = [0, 2]
-            a[fields][ix] = v[fields][ix]
-            z.set_coordinate_selection(ix, v[fields][ix], fields=fields)
+            a[key][ix] = v[key][ix]
+            z.set_coordinate_selection(ix, v[key][ix], fields=fields)
             assert_array_equal(a, z[:])
 
             # mask selection
             a[:] = ('', 0, 0)
             z[:] = ('', 0, 0)
             ix = [True, False, True]
-            a[fields][ix] = v[fields][ix]
-            z.set_mask_selection(ix, v[fields][ix], fields=fields)
+            a[key][ix] = v[key][ix]
+            z.set_mask_selection(ix, v[key][ix], fields=fields)
             assert_array_equal(a, z[:])