Repository navigation
Request for fancy indexing #657
Description
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]] = 1Results 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.0numcodecs.__version__--> 0.7.2- Version of Python interpreter --> 3.8.5
- Operating system --> Mac
- How Zarr was installed --> conda
Reacted by Volker Hilsenstein
Activity
jakirkham commentedon Nov 24, 2020
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 orvindexandoindexfor 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 commentedon Nov 24, 2020
Thanks @jakirkham!
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
.vindexin 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 commentedon Mar 3, 2021
@zarr-developers/core-devs, would be good to hear from people on whether this change sounds reasonable
shoyer commentedon Mar 3, 2021
rabernat commentedon Mar 3, 2021
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 commentedon Mar 3, 2021
I think it currently errors
jakirkham commentedon Mar 3, 2021
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 commentedon Mar 3, 2021
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 commentedon Mar 3, 2021
^ @jni
jni commentedon Mar 3, 2021
We're ok, we don't do the crazy combining stuff. 😜 I think! 😅
Thanks @shoyer, we'll try to submit a PR soon!
Fall back on .vindex when basic indexing fails
Fall back on .vindex when basic indexing fails
Fall back on .vindex when basic indexing fails
Fall back on .vindex when basic indexing fails
Fall back on .vindex when basic indexing fails
Add support for fancy indexing on get/setitem (#725)
jni commentedon Oct 20, 2021
Closed by #725! 🎉