Skip to content

is_total_slice doesn't handle integer indexes correctly #1730

Closed
@rabernat

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

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