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

Link to sort! in the docs of searchsorted* #48449

Merged
merged 3 commits into from
Jan 31, 2023

Conversation

jishnub
Copy link
Contributor

@jishnub jishnub commented Jan 30, 2023

The keyword arguments are explained in sort!, and it would be good to know where to look. Also, I've added an example of how to use the by keyword argument in each case.

The keyword arguments are explained in `sort!`, and it would be good to know where to look. Also, I've added an example of how to use the `by` keyword argument in each case.
@jishnub jishnub added the docs This change adds or pertains to documentation label Jan 30, 2023
@jmichel7
Copy link

The examples of searchsorted with a function by are misleading. The fact is that contrary to intuition the function by is applied to the second argument. In the call
searchsortedlast([1=>"one", 2=>"two", 4=>"four"], 3, by=first)
it works because first(3)==3. But the call
searchsortedlast([:a=>"one", :b=>"two", :d=>"four"], :c, by=first)
would not work. The proper second argument should be a pair like :c=>0.

@jishnub
Copy link
Contributor Author

jishnub commented Jan 30, 2023

Thanks! That's a good catch. I've updated the examples now

Copy link
Member

@LilithHafner LilithHafner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@knuesel has included very similar changes in #48387, but I prefer your example usage with first over @knuesel's example of abs because I think first is more likely to be used in practice.

I'd still like to hear @knuesel's thoughts on this.

@jishnub
Copy link
Contributor Author

jishnub commented Jan 30, 2023

Ah, I had missed that PR. Since that is more comprehensive, perhaps this can be closed in favor of that after a decision on the example has been taken

@knuesel
Copy link
Member

knuesel commented Jan 31, 2023

I also like @jishnub's example. Maybe we can merge this already and I'll remove the abs example when I update my draft PR?

@LilithHafner LilithHafner merged commit 768d537 into JuliaLang:master Jan 31, 2023
@LilithHafner LilithHafner added the search & find The find* family of functions label Jan 31, 2023
@jishnub jishnub deleted the patch-8 branch February 1, 2023 02:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs This change adds or pertains to documentation search & find The find* family of functions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants