Clarify that boolean array indexing requirements #164
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
We only require that boolean array indexing be supported when the array is the sole index. The previous text was ambiguous, and could be read as supporting boolean arrays plus other non-array indices as part of a larger multidimensional index. However, this is complicated in general, as it can lead to some corner cases. See the discussion in data-apis#84. With that being said, it is still an open discussion whether we should allow some subset of boolean array indices along with other non-array indices in the same index. I believe regardless, what we do not want to allow is: - multiple boolean arrays (this requires broadcasting semantics, which are confusing for boolean arrays) - boolean arrays and integers separated by a non-integer index (for example a[b, :, 0] where b is a boolean array). This is the corner case currently implemented by NumPy which we want to avoid for the standard.
kgryte
approved these changes
Apr 15, 2021
rgommers
approved these changes
Apr 19, 2021
There was a problem hiding this comment.
This looks like a useful textual clarification for what the current version of the text aims to express, so let's merge it.
With that being said, it is still an open discussion whether we should allow
some subset of boolean array indices along with other non-array indices in the
same index. I believe regardless, what we do not want to allow is:
Agreed. Let's continue that conversation on gh-84.
|
Thanks @asmeurer |
rgommers
added
Narrative Content
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We only require that boolean array indexing be supported when the array is the
sole index. The previous text was ambiguous, and could be read as supporting
boolean arrays plus other non-array indices as part of a larger
multidimensional index. However, this is complicated in general, as it can
lead to some corner cases. See the discussion in #84.
With that being said, it is still an open discussion whether we should allow
some subset of boolean array indices along with other non-array indices in the
same index. I believe regardless, what we do not want to allow is:
confusing for boolean arrays)
a[b, :, 0] where b is a boolean array). This is the corner case currently
implemented by NumPy which we want to avoid for the standard.