zarr-developers / zarr-python

An implementation of chunked, compressed, N-dimensional arrays for Python.
http://zarr.readthedocs.io/
MIT License
1.43k stars 272 forks source link

Feedback on v3 Array setitems behavior with single items #1923

Open rabernat opened 2 months ago

rabernat commented 2 months ago

I have been playing with the v3 branch and noticed some weird behavior with setting values.

import zarr
arr = zarr.Array.create(store=zarr.store.MemoryStore(), shape=(10,), chunks=(5,), dtype="i4")

# this errors with v3 but works with v2
arr[0] = 1

# this works
arr[:1] = [1]

I think it's pretty critical for this style of assignment to work with v3.

Error:

In [5]: arr[0] = 1
---------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)
Cell In[5], line 1
----> 1 arr[0] = 1

File ~/mambaforge/envs/base311/lib/python3.11/site-packages/zarr/array.py:591, in Array.__setitem__(self, selection, value)
    590 def __setitem__(self, selection: Selection, value: NDArrayLike) -> None:
--> 591     sync(
    592         self._async_array.setitem(selection, value),
    593     )

File ~/mambaforge/envs/base311/lib/python3.11/site-packages/zarr/sync.py:92, in sync(coro, loop, timeout)
     89 return_result = next(iter(finished)).result()
     91 if isinstance(return_result, BaseException):
---> 92     raise return_result
     93 else:
     94     return return_result

File ~/mambaforge/envs/base311/lib/python3.11/site-packages/zarr/sync.py:51, in _runner(coro)
     46 """
     47 Await a coroutine and return the result of running it. If awaiting the coroutine raises an
     48 exception, the exception will be returned.
     49 """
     50 try:
---> 51     return await coro
     52 except Exception as ex:
     53     return ex

File ~/mambaforge/envs/base311/lib/python3.11/site-packages/zarr/array.py:401, in AsyncArray.setitem(self, selection, value, factory)
    395 async def setitem(
    396     self,
    397     selection: Selection,
    398     value: NDArrayLike,
    399     factory: Factory.NDArrayLike = NDBuffer.from_ndarray_like,
    400 ) -> None:
--> 401     indexer = BasicIndexer(
    402         selection,
    403         shape=self.metadata.shape,
    404         chunk_grid=self.metadata.chunk_grid,
    405     )
    407     sel_shape = indexer.shape
    409     # check value shape

File ~/mambaforge/envs/base311/lib/python3.11/site-packages/zarr/indexing.py:144, in BasicIndexer.__init__(self, selection, shape, chunk_grid)
    140 assert isinstance(
    141     chunk_grid, RegularChunkGrid
    142 ), "Only regular chunk grid is supported, currently."
    143 # setup per-dimension indexers
--> 144 self.dim_indexers = [
    145     _SliceDimIndexer(dim_sel, dim_len, dim_chunk_len)
    146     for dim_sel, dim_len, dim_chunk_len in zip(
    147         _ensure_selection(selection, shape), shape, chunk_grid.chunk_shape, strict=False
    148     )
    149 ]
    150 self.shape = tuple(s.nitems for s in self.dim_indexers)

File ~/mambaforge/envs/base311/lib/python3.11/site-packages/zarr/indexing.py:145, in <listcomp>(.0)
    140 assert isinstance(
    141     chunk_grid, RegularChunkGrid
    142 ), "Only regular chunk grid is supported, currently."
    143 # setup per-dimension indexers
    144 self.dim_indexers = [
--> 145     _SliceDimIndexer(dim_sel, dim_len, dim_chunk_len)
    146     for dim_sel, dim_len, dim_chunk_len in zip(
    147         _ensure_selection(selection, shape), shape, chunk_grid.chunk_shape, strict=False
    148     )
    149 ]
    150 self.shape = tuple(s.nitems for s in self.dim_indexers)

File ~/mambaforge/envs/base311/lib/python3.11/site-packages/zarr/indexing.py:70, in _SliceDimIndexer.__init__(self, dim_sel, dim_len, dim_chunk_len)
     69 def __init__(self, dim_sel: slice, dim_len: int, dim_chunk_len: int):
---> 70     self.start, self.stop, self.step = dim_sel.indices(dim_len)
     71     if self.step < 1:
     72         _err_negative_step()

AttributeError: 'int' object has no attribute 'indices'
jhamman commented 2 months ago

I'll let @normanrz say for sure but this should be handled by #1917

normanrz commented 2 months ago

Yes, this should be fixed in #1917