-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Rename findmin and findmax? #24865
Comments
Isn't this basically what is usually called argmin and argmax (although those are not great names)? |
And indeed: #1655. |
I still like |
For the method returning just the index (current |
Do those need to be separate methods? Is there a cost to always just returning both the index and the value? |
As I noted in the OP, there's a "consistency cost" (if that's a thing) since |
I think we may have to hammer this one out with @JeffBezanson when he gets back as part of #10593. I hate to break the deadline, but I think this needs his input. It will be easier to work though after we've gotten all the other stuff out of the way. |
I'm ok with renaming Maybe we could call it I also think this is low-urgency and doesn't need to be done for 1.0. |
FWIW, we have a series of functions with "and" in StatsBase: We could also find a different term to indicate the action of returning both the value and the index: |
Or |
I'd prefer that we general move in that direction and return the value in the |
That's a possibility, but that's going to be much more breaking that what I already have as part of #10593. That would also introduce an inconsistency with I think it would be simpler to have the |
From triage: does not need to be in 1.0. |
#25654 renames |
Julia user suggestion: please save others time by mentioning in docs for
Something like "For multi-dimensional arrays, the |
|
Note that |
Seems like we settled on something, given that 1.0 is not accepting deprecations anymore. If someone disagrees, post a comment on the further change needed. |
findmin
andfindmax
return a tuple with the index of the maximum/minimum, and the corresponding value. This behavior is kind of inconsistent withfind
,findfirst
,findlast
,findnext
andfindprev
, which return indices without values. On the other hand,indmin
andindmax
return only the indices.It would be more consistent to rename
indmin
/indmax
tofindmin
/findmax
. Of course this is not possible in a single deprecation cycle. I can see two solutions:findmin
andfindmax
in 0.7, and renameindmin
andindmax
tofindmin
andfindmax
in 1.0. This has the drawback of breaking things between 0.7 and 1.0.indmin
andindmax
tofindmin
andfindmax
in 0.7. This introduces breaking changes in 0.7, but at least there wouldn't be any changes in 1.0.Finally, it's not clear to me whether we really need the current behavior of
findmin
/findmax
, compared with getting the index and then extracting the corresponding element manually. The performance gain should only be significant for very small collections AFAICT.The text was updated successfully, but these errors were encountered: