ENH: add nanmin#804
Conversation
|
@lucascolley I added one of these for a start. I think after we finalize one of them the others can follow along. |
lucascolley
left a comment
lucascolley
left a comment
There was a problem hiding this comment.
there are a few merge conflicts
| return a | ||
|
|
||
|
|
||
| def nanmin( # numpydoc ignore=PR01,RT01 |
There was a problem hiding this comment.
whereabouts did you take this implementation from?
There was a problem hiding this comment.
scikit-learn directly. As for how that is implemented there I am not exactly sure.
There was a problem hiding this comment.
would be good to check the history there
There was a problem hiding this comment.
https://github.com/scikit-learn/scikit-learn/pull/26243/changes
This is the actual PR
There was a problem hiding this comment.
thanks — @betatim would you mind weighing in on how this implementation arose?
EDIT: /feel free to review also :)
There was a problem hiding this comment.
I think this mainly follows the numpy implementation ignoring cases of objects or ndarray specifics:
https://github.com/numpy/numpy/blob/main/numpy/lib/_nanfunctions_impl.py#L252-L374
There was a problem hiding this comment.
What exactly are you looking for in terms of histroy Lucas?
My guess is that like Omar said it is based on what is in Numpy, potentially minus some extra args/handling of edge cases.
There was a problem hiding this comment.
What exactly are you looking for in terms of histroy Lucas?
My guess is that like Omar said it is based on what is in Numpy, potentially minus some extra args/handling of edge cases.
If that's the case, then yeah, I'll start by looking at what NumPy does
There was a problem hiding this comment.
Yeah, the way these functions make their way into scikit-learn is "this exists in numpy, doesn't exist in array API, we need it, let's translate the numpy code"
3 participants
Towards: #789
nanmin