Skip to content

is_total_slice doesn't handle integer indexes correctly #1730

Description

@rabernat

Zarr version

all of them

Numcodecs version

irrelevant

Python Version

irrelevant

Operating System

irrelevant

Installation

irrelevant

Description

The function zarr.utils.is_total_slice is used to determine if we can completely replace a chunk or if we need to first read it an merge it. Reading and merging is much more expensive than just overwriting, so this is a critical performance optimization.

There is a bug in this logic in the case of a chunk size of 1 (along any dimension). In this case indexing with an integer is the same as indexing with a length-1 slice. i.e. these two indexing operations are equivalent

a[0]
a[slice(0, 1)]

Unfortunately this function treats those two cases as different, incorrectly returning False for the former but True for the latter.

Steps to reproduce

import zarr

# the first argument is the selection, the second argument is the chunk shape
assert zarr.util.is_total_slice((slice(0, 1),), (1,))
# this is an equivalent selection (without a slice) - currently evaluates to False
assert not zarr.util.is_total_slice((0,), (1,)) 
# same for multidimensional selection
assert zarr.util.is_total_slice((slice(0, 1), slice(0, 10)), (1, 10))
assert not zarr.util.is_total_slice((0, slice(0, 10)), (1, 10))

Additional output

No response

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugPotential issues with the zarr-python library

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions