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

RFC: seachsorted(a,x,by=f) no longer applies by to the key argument x #16056

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

krcools
Copy link

@krcools krcools commented Apr 26, 2016

Following #9429, this pull request modifies searchsorted and friends to not apply to optional by argument to the sought for key x. As explained in #9429, this makes sense because it is not always possible to come up with a value x such that by(x) is the determinant is the binary search. In addition to the examples in #9429, consider a line of people ordered according to height. It is possible that a user is interested in the first person taller then 1.66 meters, even though no person in the line-up actually has that height.

The behaviour of searchsorted before these modifications can be achieved by calling

searchsorted(a,f(x),by=f)

The tests in test/sorting.jl have been modified to pass. In particular, consider:

@test searchsorted([1:10], false, by=(x -> x >=5)) == 1:4

Before, this test read

@test searchsorted([1:10], 1, by=(x-> x >=5)) == 1:4

This demonstrates how unnatural the previous behaviour was. The key 1 is arbitrarily chosen from the set of values that give by(x) == false. In addition, the previous behaviour obfuscates that in order for searchsorted to work correctly, it relies on the Julia convention false < true.

@StefanKarpinski
Copy link
Member

I don't think this implementation will work well when combined with other ordering mechanisms. This all needs revisiting in light of jb/functions. The Ordering objects are no longer necessary – you can just pass in a comparison function directly, instead.

@krcools
Copy link
Author

krcools commented Apr 27, 2016

I'm not fixated on any implementation but I do think the semantics of searchsorted should change from what they are now to what is proposed above.

Could you refer me to the relevant issues/other reading on jb/functions?

@nalimilan nalimilan added the search & find The find* family of functions label Nov 29, 2017
@ViralBShah ViralBShah added sorting Put things in order and removed sorting Put things in order labels Jul 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
search & find The find* family of functions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants