Conversation
Signed-off-by: Iason Krommydas <[email protected]>
|
There is one more call site that could be replaced in C in |
Signed-off-by: Iason Krommydas <[email protected]>
np.minmaxnp.minmax
So just to verify, the Python |
Yes and I’ve edited it here. It’s one of those functions that call a method on the ndarray that is implemented in python |
Signed-off-by: Iason Krommydas <[email protected]>
| min_value, max_value = umr_minimummaximum( | ||
| a, axis, None, (None, out), keepdims | ||
| ) | ||
| except TypeError: |
There was a problem hiding this comment.
@ngoldbaum I'm pretty sure we have to use TypeError here and not _UFuncNoLoopError only because here we also want to catch __array_ufunc__ of ndarray subclasses rejecting minmax and we want to route that to the fallback. In np.minmax we didn't because the argument was "just use np.min/max separately manually if your __array_ufunc__ doesn't support it" but here it's inside another function that the user has no control over so we have to steer it to the fallback ourselves.
|
Writing it here for visibility, there are a few edge cases regarding legacy user dtypes and array ufunc that we need to take care of here before proceeding with this. Will bring it up in the next numpy meeting. |
|
Thanks Iason for all of your work on implementing and using Would it make sense to optimize this one as well? numpy/numpy/_core/src/multiarray/calculation.c Lines 284 to 291 in d8c7dad |
|
Thanks @jakirkham . I wrote about this place earlier actually in #32577 (comment) |
PR summary
Since #32231 has been merged, we replace consecutive min and max calls with the new
np.minmax. This is a refactoring PR and functionality does not change. This also gives the performance benefit of doing a single pass in those cases.AI Disclosure
An AI model has been used to conduct an AST search for such location in the source code to make sure I did not miss any by eye.