BUG,DEP: Allow (arg-)partition to accept uint64 indices
#20000
Conversation
There was a problem hiding this comment.
This looks good to me, thanks! I am happy with deprecating bools as well, since the new behaviour would mirror indexing.
(Just in case someone wonders: The casting can lead to integer overflows during the cast in theory, this is something we have to live with right now unfortunately, we do the same in indexing.)
| return NULL; | ||
| } | ||
| } | ||
| else if (!PyArray_ISINTEGER(ktharray)) { |
There was a problem hiding this comment.
OK, I think we may have to switch hat over to a same-kind casting (possibly with a time exception). But it is what indexing currently uses, so I think it is fine. (This would not allow custom user integer DTypes.)
There was a problem hiding this comment.
For future refernce: I wonder what would be the best way of dealing with custom user integer DTypes?
Considering PyArray_ISINTEGER is too strict and np.can_cast(dtype, np.intp, casting="same_kind") won't even accept all the standard numpy integer dtypes (i.e. np.uint64).
EDIT: Updated the can_cast example
There was a problem hiding this comment.
All NumPy integers should pass fine? The main problem is that bools and datetimes may also. If same-kind casting doesn't do the trick, we could also consider adding an abstract Indexer DType that users have to register or implement promotion with.
There was a problem hiding this comment.
All NumPy integers should pass fine?
Ah, to clarify: I was talking specifically about np.uint64 in the can_cast example.
If same-kind casting doesn't do the trick, we could also consider adding an abstract
IndexerDType that users have to register or implement promotion with.
I could see this potentially being usefull, as this would also make it easier to differentiate between (user-defined) bool dtypes.
There was a problem hiding this comment.
In [1]: np.can_cast(np.uint64, np.int32, casting="same_kind")
Out[1]: True
There was a problem hiding this comment.
Ugh, you're completelly right.
I may have accidently mixed up "safe" and "same_kind" in my head...
There was a problem hiding this comment.
But you are right, an abstract IndexingDType may be a good way to add some useful formalism and better generalization.
… warning Co-Authored-By: Sebastian Berg <sebastian@sipsolutions.net>
|
Thanks Bas, if no-one else has any concerns, lets put this in! (I suppose we should probably ping the mailing list very briefly about this, but I feel it is just a formality.) |
Closes #19832
This PR introduces two changes:
uint64indices, an issue caused by what was effectively anp.can_cast(kth, np.intp, casting="same_kind")check.np.intpand thus treating them as normal numerical indices. The passing of booleans to thekthparameter has now been deprecated.