Skip to content
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 searchsorted* functions to findsorted* #25414

Closed
wants to merge 1 commit into from
Closed

Conversation

nalimilan
Copy link
Member

@nalimilan nalimilan commented Jan 5, 2018

The name is consistent with the new API, as search no longer exists.
Also swap the two positional arguments for consistency with other functions.

Alternative approach to fix #24883. Replaces #25133, part of #10593.

The name is consistent with the new API, as search() no longer exists.
Also swap the two positional arguments for consistency with other functions.
@JeffBezanson
Copy link
Sponsor Member

One reason I prefer the old names is that they implement "binary search", which is a commonly-recognized term.

@nalimilan
Copy link
Member Author

Then they should be called binarysearch, not searchsorted. There's also the annoyance that the arguments are in reverse order compared with other functions.

@mschauer
Copy link
Contributor

mschauer commented Jan 5, 2018

findsorted suggests the function looks for a sorted (sub-)sequence, if the name is an analogy to findmax (finding the maximum), findlast (finding the last occurence) etc.

@nalimilan nalimilan added the status:triage This should be discussed on a triage call label Jan 10, 2018
@nalimilan
Copy link
Member Author

For triage: the question of the name is somewhat secondary, what concerns me more is the fact that the order of arguments of the searchsorted* functions is currently the opposite of the find* functions. So there are two decisions to make:

  • should we change the order of arguments for consistency
  • should we use the occasion to rename the functions too

@StefanKarpinski
Copy link
Sponsor Member

StefanKarpinski commented Jan 11, 2018

My feeling is that we should just leave these alone. If we ever come up with a better coherent API using find functions and SortedVector types, that would be great, but at that point, we can just introduce that and have these as a legacy API until 2.0 comes around. (This seems to be the general sentiment from triage as well.)

@StefanKarpinski StefanKarpinski removed the status:triage This should be discussed on a triage call label Jan 11, 2018
@nalimilan nalimilan deleted the nl/findsorted branch January 19, 2018 17:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
domain:search & find The find* family of functions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

What to do with searchsorted* functions
4 participants