Skip reads when completely overwriting boundary chunks

Uses `slice(..., None)` to indicate that a `chunk_selection`
ends at the boundary of the current chunk. Also does so for a last
chunk that is shorter than the chunk size.

`is_total_slice` now understands this convention, and correctly
detects boundary chunks as total slices.

Closes zarr-developers#757

Author: dcherian

src/zarr/core/indexing.py

@@ -403,16 +403,24 @@ def __iter__(self) -> Iterator[ChunkDimProjection]:
                 dim_chunk_sel_start = self.start - dim_offset
                 dim_out_offset = 0
 
+            # Use None to indicate this selection ends at the chunk edge
+            # This is useful for is_total_slice at boundary chunks,
             if self.stop > dim_limit:
                 # selection ends after current chunk
-                dim_chunk_sel_stop = dim_chunk_len
+                dim_chunk_sel_stop = None  # dim_chunk_len
 
             else:
                 # selection ends within current chunk
-                dim_chunk_sel_stop = self.stop - dim_offset
+                if dim_chunk_ix == (self.nchunks - 1):
+                    # all of the last chunk is included
+                    dim_chunk_sel_stop = None
+                else:
+                    dim_chunk_sel_stop = self.stop - dim_offset
 
             dim_chunk_sel = slice(dim_chunk_sel_start, dim_chunk_sel_stop, self.step)
-            dim_chunk_nitems = ceildiv((dim_chunk_sel_stop - dim_chunk_sel_start), self.step)
+            dim_chunk_nitems = ceildiv(
+                ((dim_chunk_sel_stop or dim_chunk_len) - dim_chunk_sel_start), self.step
+            )
 
             # If there are no elements on the selection within this chunk, then skip
             if dim_chunk_nitems == 0:
@@ -1378,7 +1386,17 @@ def is_total_slice(item: Selection, shape: ChunkCoords) -> bool:
                 isinstance(dim_sel, slice)
                 and (
                     (dim_sel == slice(None))
-                    or ((dim_sel.stop - dim_sel.start == dim_len) and (dim_sel.step in [1, None]))
+                    or (
+                        dim_sel.stop is not None
+                        and (dim_sel.stop - dim_sel.start == dim_len)
+                        and (dim_sel.step in [1, None])
+                    )
+                    # starts exactly at a chunk
+                    or (
+                        (dim_sel.start % dim_len == 0)
+                        and dim_sel.stop is None
+                        and (dim_sel.step in [1, None])
+                    )
                 )
             )
             for dim_sel, dim_len in zip(item, shape, strict=False)

src/zarr/storage/_logging.py

@@ -88,7 +88,7 @@ def log(self, hint: Any = "") -> Generator[None, None, None]:
         op = f"{type(self._store).__name__}.{method}"
         if hint:
             op = f"{op}({hint})"
-        self.logger.info("Calling %s", op)
+        self.logger.info(" Calling %s", op)
         start_time = time.time()
         try:
             self.counter[method] += 1

tests/test_array.py

@@ -1335,5 +1335,14 @@ async def test_orthogonal_set_total_slice() -> None:
     """Ensure that a whole chunk overwrite does not read chunks"""
     store = MemoryStore()
     array = zarr.create_array(store, shape=(20, 20), chunks=(1, 2), dtype=int, fill_value=-1)
-    with mock.patch("zarr.storage.MemoryStore.get", side_effect=ValueError):
+    with mock.patch("zarr.storage.MemoryStore.get", side_effect=RuntimeError):
         array[0, slice(4, 10)] = np.arange(6)
+
+    array = zarr.create_array(
+        store, shape=(20, 21), chunks=(1, 2), dtype=int, fill_value=-1, overwrite=True
+    )
+    with mock.patch("zarr.storage.MemoryStore.get", side_effect=RuntimeError):
+        array[0, :] = np.arange(21)
+
+    with mock.patch("zarr.storage.MemoryStore.get", side_effect=RuntimeError):
+        array[:] = 1

tests/test_indexing.py

@@ -1959,3 +1959,6 @@ def test_vectorized_indexing_incompatible_shape(store) -> None:
 def test_is_total_slice():
     assert is_total_slice((0, slice(4, 6)), (1, 2))
     assert is_total_slice((slice(0, 1, None), slice(4, 6)), (1, 2))
+    assert is_total_slice((slice(0, 1, None), slice(4, None)), (1, 2))
+    # slice(5, None) starts in the middle of a chunk
+    assert not is_total_slice((slice(0, 1, None), slice(5, None)), (1, 2))