Request for fancy indexing

[AbigailMcGovern]

Overview

Attempting to modify/retrive values in a zarr array with fancy indexing results in an error.

Minimal, reproducible code sample

For instance, a tuple of list indices

import zarr
array  = zarr.zeros((20, 20))
array[[1, 2, 3], [1, 2, 3]] = 1

Results in the following:

IndexError: unsupported selection item for basic indexing; expected integer or slice, got <class 'list'>

However, this works usingvindex

array.vindex[[1, 2, 3], [1, 2, 3]] = 1
print(array.vindex[[1, 2, 3], [1, 2, 3]])
# Output: [1. 1. 1.]

Problem description

Zarr's Array class does not support fancy indexing via the standard numpy-like syntax, which causes issues when attempting to write to zarr arrays via napari's label layer. These problems arise because several of the label editing tools in napari (e.g., paintbrush, fill tool, etc) require fancy indexing. @jni and myself are hoping that the logic in vindex can be incorporated to Array's __setitem__ and __getitem__. Any advice you can offer in this regard would be much appreciated!

Version and installation information

• zarr.__version__ --> 2.4.0
• numcodecs.__version__ --> 0.7.2
• Version of Python interpreter --> 3.8.5
• Operating system --> Mac
• How Zarr was installed --> conda

[jakirkham]

Advanced indexing is covered in these docs.

That said, we wound up not adding these to __getitem__ and __setitem__ as folks found this syntax a bit confusing to parse and distinguish from the different types of fancy indexing. Instead we added custom methods or vindex and oindex for clarity, which seems to follow Dask, a NEP, and some other projects that have gone down this road. For more context this functionality was added in PR ( #172 ).

[jni]

Thanks @jakirkham!

Instead we added custom methods or vindex and oindex for clarity, which seems to follow Dask, a NEP, and some other projects that have gone down this road

The problem is that NumPy itself has not, at least not yet, so in order for napari to support this we would have to implement different code for different array libraries. Fancy indexing is standard NumPy behaviour and it seems to me that, since it's already implemented, it should be straightforward to detect whether a tuple of arrays is given, and do .vindex in that case?

Note: I do think the case of mixing slices and arrays is confusing, but that implementation could be skipped for now? ie one can strictly increase the number of supported use cases without having to implement a whole stack of new functionality?

[jakirkham]

@zarr-developers/core-devs, would be good to hear from people on whether this change sounds reasonable

[shoyer]

I think it would be reasonable to copy NumPy's behavior for fancy indexing. It is indeed unintuitive, but not hard to copy if you explicitly use the same rules.

…

On Tue, Mar 2, 2021 at 4:31 PM jakirkham ***@***.***> wrote: @zarr-developers/core-devs <https://github.com/orgs/zarr-developers/teams/core-devs>, would be good to hear from people on whether this change sounds reasonable — You are receiving this because you are on a team that was mentioned. Reply to this email directly, view it on GitHub <#657 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAJJFVTAZDCIKSHGTDOJM3LTBV7PPANCNFSM4UAFHSKA> .

[rabernat]

Would this constitute a breaking change? Or just something that used to error that now would work? If the latter, I am 👍. People don't have to use it if they find it confusing!

[jakirkham]

I think it currently errors

[jakirkham]

For context this came up as it is handy in duck-typed code that may handle many NumPy-like things (like Dask, Zarr, etc.).

[shoyer]

For context this came up as it is handy in duck-typed code that may handle many NumPy-like things (like Dask, Zarr, etc.).

To be clear, Dask is not yet current consistent with NumPy, either, but that could be fixed: dask/dask#7313. Thus I would advise caution before relying on this feature :)

[jakirkham]

^ @jni

[jni]

We're ok, we don't do the crazy combining stuff. 😜 I think! 😅

Thanks @shoyer, we'll try to submit a PR soon!

[jni]

Closed by #725! 🎉