Repository navigation
TYP: Fix falsely rejected value types in ndarray.__setitem__ - #27967
Conversation
jorenham
added
00 - Bug
09 - Backport-Candidate
seberg
left a comment
seberg
left a comment
There was a problem hiding this comment.
This change looks fine to me (and introducing the _ToIndices seems nice).
But, I am not quite sure about the datetime definition.
I can't say whether it is all the ideal definitions, so if anyone else deeper into typing wants to have a look that would be nice.
| else: | ||
| _ConvertibleToComplex: TypeAlias = complex | SupportsComplex | SupportsFloat | SupportsIndex | _CharLike_co | ||
| _ConvertibleToTD64: TypeAlias = dt.timedelta | int | _CharLike_co | character | number | timedelta64 | np.bool | None | ||
| _ConvertibleToDT64: TypeAlias = dt.date | int | _CharLike_co | character | number | datetime64 | np.bool | None |
There was a problem hiding this comment.
How come this has character and _CharLike_co? Is that intentional?
Is this missing dt.datetime?
There was a problem hiding this comment.
Yea, it's indeed missing in datetime64:
>>> np.datetime64(np.str_("NaT"))
np.datetime64('NaT')
>>> np.datetime64(np.bytes_("NaT"))
np.datetime64('NaT')but also
>>> np.datetime64(np.str_("42"))
np.datetime64('0042')
>>> np.datetime64(np.bytes_("42"))
np.datetime64('0042')🤷🏻
I could sneak it in this PR, or open another PR for it?
There was a problem hiding this comment.
I could sneak it in this PR, or open another PR for it?
Not sure I care, if you think it's simple enough (IIUC, this is about the scalar creation?). FWIW, I would expect them to use the identical definitions to scalar assignment.
W.r.t to the strings, what surprises me was that _ConvertibleToComplex does not include character.
The other thing is that you have _ConvertibleToDT64: TypeAlias = dt.date. Should it not also have dt.datetime?
There was a problem hiding this comment.
W.r.t to the strings, what surprises me was that
_ConvertibleToComplexdoes not includecharacter.
>>> np.str_.__float__
<slot wrapper '__float__' of 'numpy.generic' objects>which is also reflected in
Line 1648 in fc539ff
so it's already sneakily included
The other thing is that you have
_ConvertibleToDT64: TypeAlias = dt.date. Should it not also havedt.datetime?
dt.datetime subclasses of dt.date, so it's automatically included
There's a good chance I'm missing something here, so more eyes could indeed help in this case |
|
Let's get this in. I doubt it will be the last :) Thanks Joren. |
|
Thanks for the fix! |
I missed something... ... and it's now causing the scipy CI to fail 😅 |
fixes #27964
cc @jakevdp