Skip to content

BUG: Make sure that NumPy scalars are supported by can_cast #26372

Merged
charris merged 2 commits intonumpy:mainfrom
seberg:can-cast-numpy-scalar
May 6, 2024
Merged

BUG: Make sure that NumPy scalars are supported by can_cast #26372
charris merged 2 commits intonumpy:mainfrom
seberg:can-cast-numpy-scalar

Conversation

@seberg
Copy link
Member

@seberg seberg commented May 2, 2024

The main issue here was the order of the checks, since float64 is
a subclass of float the error path was taken even though it should
not have been.

This also avoids converting to an array (which is very slow) when
possible. I opted to use scalar.dtype since that may be a bit
easier for potential future user dtype.
That may not be quite ideal (I would like to not force np.generic
as a base-class for user scalars), but is probably pretty close
and more complicated fixes are probably not good for backport.

Closes gh-26370


Also adds a can_cast change to the relase notes as it was missing.

seberg added 2 commits May 2, 2024 13:43
The main issue here was the order of the checks, since float64 is
a subclass of float the error path was taken even though it should
not have been.

This also avoids converting to an array (which is very slow) when
possible.  I opted to use `scalar.dtype` since that may be a bit
easier for potential future user dtype.
That may not be quite ideal (I would like to not force `np.generic`
as a base-class for user scalars), but is probably pretty close
and more complicated fixes are probably not good for  backport.
@seberg seberg added the 09 - Backport-Candidate PRs tagged should be backported label May 2, 2024
@seberg
Copy link
Member Author

seberg commented May 2, 2024

@charris do you prefer to split out the doc change, or apply this to the 2.0 branch first and forward port?

@charris
Copy link
Member

charris commented May 2, 2024

do you prefer to split out the doc change,

Hmm, it is missing a link, which you cannot know until the PR is made. I'll take care of it in the backport, but it would probably be easier in the future to split it out and submit against the branch after the backport goes in.

@seberg
Copy link
Member Author

seberg commented May 2, 2024

Ah right, I could just add a link to this PR (and try to find some link to something older to include here)?
I think otherwise, it'll point to some larger very old NEP 50 PR. I mean the link should be a bread-crumb to find something interesting, maybe this isn't so bad :).

@seberg
Copy link
Member Author

seberg commented May 6, 2024

It may be nice if we could put this in and aim for a rc2 soon? E.g. the fft bug was hitting some downstream testers and it may be nice to resolve that.
Unless there is something else that would be nice to definitely include in rc2?

@charris
Copy link
Member

charris commented May 6, 2024

It may be nice if we could put this in and aim for a rc2 soon?

I was thinking next weekend so there would be a version out there with fixes. Could be earlier if needed.

@charris charris merged commit dd33199 into numpy:main May 6, 2024
@charris
Copy link
Member

charris commented May 6, 2024

Thanks Sebastian.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

BUG: can_cast must support NumPy scalars

2 participants