Repository navigation
BUG: NEP 50 promotion for python scalars in ufunc.outer - #32090
Conversation
Signed-off-by: Iason Krommydas <[email protected]>
ikrommyd
changed the title
ufunc.outer
Signed-off-by: Iason Krommydas <[email protected]>
Signed-off-by: Iason Krommydas <[email protected]>
|
I'd argue that my last commit makes the code more readable but moves things around a little more. Happy to adjust but I generally prefer more readable control flow. |
ngoldbaum
left a comment
ngoldbaum
left a comment
There was a problem hiding this comment.
Overall looks good, just a few minor issues. Thanks for taking this on :)
mhvk
left a comment
mhvk
left a comment
There was a problem hiding this comment.
Nice! Two really small comments only.
Signed-off-by: Iason Krommydas <[email protected]>
|
I think I have addressed both reviews :) |
ngoldbaum
left a comment
ngoldbaum
left a comment
There was a problem hiding this comment.
I think this is good and I also think it's probably fine to just change this and #32094 in the release without a deprecation to make NumPy more consistent.
That said, I'd appreciate it if @seberg could confirm - can we get away without a deprecation cycle like we did for the "main" NEP-50 transition?
mhvk
left a comment
mhvk
left a comment
There was a problem hiding this comment.
Looks good! Still a tiny suggestion of nomenclature, but approving now.
Signed-off-by: Iason Krommydas <[email protected]>
As a user, I probably wouldn't want this backported cause the change of behavior for large python integers might surprise some people but for 2.6.0 I wouldn't complain at all. All that is with my user's hat on. |
|
Sebastian told me to stop worrying about getting his eyes on stuff. In it goes! |
PR summary
Closes #32076
ufunc.outerconverted its inputs to arrays before the ufunc machinery saw them, so Python scalars lost their NEP 50 weak typing:np.add.outer(1.0, f32_arr)gave float64 wherenp.add(1.0, f32_arr)gives float32.Scalars are now passed on unconverted. Huge Python integers now raise
OverflowErrorhere, matchingufunc.__call__.First time committer introduction
N/A
AI Disclosure
Ran it through AI for review and it only touched the wording on the release note.