Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Specify that dunder get/set item accepts SupportsIndex #383

Open
honno opened this issue Jan 25, 2022 · 2 comments · May be fixed by #766
Open

Specify that dunder get/set item accepts SupportsIndex #383

honno opened this issue Jan 25, 2022 · 2 comments · May be fixed by #766
Labels
Maintenance Bug fix, typo fix, or general maintenance. topic: Indexing Array indexing.

Comments

@honno
Copy link
Member

honno commented Jan 25, 2022

Currently __getitem__()/__setitem__() accept array as keys, which implicitly has to be boolean arrays, or 0d integer arrays i.e. has a working __index__(). The latter is not an obvious concept to grasp, and maybe not even quite specified anyway.

I propose we add the stdlib typing.SupportsIndex to the get/set item signatures, specifically before array. This does add support for any indexable type, but that seems okay, and was the intention anyway when __index__() was proposed in #231. I also think a note would be helpful in regards to what arrays are accepted, e.g.:

key can only be an array if it is valid for boolean array indexing, or supports __index__() i.e. is a 0-dimensional integer array.

cc @kgryte @asmeurer
@asmeurer
Copy link
Member

Just to be clear, the point of __index__ isn't just to support 0d integer arrays as indices on arrays, but to let them be indices on any type that supports integer indexing like str and list. And conversely, for arrays to accept as integer indices any type that implements __index__ (with the exception of bool, which unfortunately inherits it from int but usually has a different indexing meaning for arrays).

i.e. is a 0-dimensional integer array.

I would omit this part. Any type can implement __index__. That's the whole point. In fact, we might link to PEP 357 instead of the spec __index__ definition.

@kgryte kgryte added Maintenance Bug fix, typo fix, or general maintenance. topic: Indexing Array indexing. labels Jan 31, 2022
@kgryte kgryte added this to the v2022 milestone Jan 31, 2022
@rgommers rgommers removed this from the v2022 milestone Dec 14, 2022
@kgryte kgryte mentioned this issue Jun 29, 2023
Closed
17 tasks
@kgryte
Copy link
Contributor

kgryte commented Mar 21, 2024

@honno Would you be willing to open a PR addressing this issue for the v2024 revision?

@kgryte kgryte added this to the v2024 milestone Mar 21, 2024
@honno honno linked a pull request Mar 21, 2024 that will close this issue
Open
@kgryte kgryte removed this from the v2024 milestone Oct 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Maintenance Bug fix, typo fix, or general maintenance. topic: Indexing Array indexing.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Replace int with SupportsIndex in indexing methods hints data-apis/array-api
4 participants
@asmeurer @rgommers @kgryte @honno