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

Move searchsorted* functions to SortedSearch stdlib module #25133

Closed
wants to merge 1 commit into from

Conversation

nalimilan
Copy link
Member

@nalimilan nalimilan commented Dec 16, 2017

Keep them under Base.Sorted but unexported, since they are used in a few places in Base, in particular for sparse vectors and matrices.

Fixes #24883.

@nalimilan nalimilan added domain:search & find The find* family of functions stdlib Julia's standard library labels Dec 16, 2017
@KristofferC
Copy link
Sponsor Member

In my opinion this feels way to specific to be a stdlib module.

@nalimilan
Copy link
Member Author

Yeah, it's not a lot of functionality for a single module. So would you recommend moving them to a package (new or existing) instead?

Keep them under Base.Sorted but unexported, since they are used
in a few places in Base, in particular for sparse vectors and matrices.
@JeffBezanson
Copy link
Sponsor Member

Why do you hate these functions so much? :)
I think we should leave them alone.

@nalimilan
Copy link
Member Author

It's just that they don't follow the naming pattern used for all find* functions, so they look really out of place (after #24673). Why use the term "search" when the API consistently uses "find" in other areas?

There's also the possibility of merging them with standard find* functions using a Sorted wrapper, whose interest hasn't been discussed yet.

@KristofferC
Copy link
Sponsor Member

If it is "just that they don't follow the naming pattern" can't they just be renamed then?

As to a sorted wrapper, if it is only used for dispatch, it seems to not solve any concrete problem but add additional ones.

@nalimilan
Copy link
Member Author

Yes, renaming them as another option. (Note that I had initially made this PR in the perspective of merging all breaking changes around December 15th, considering that it was simpler to move things to sdlib when we didn't have time to discuss these issues in depth. But since we took that design discussion time, it may not be the most appropriate action now.)

@nalimilan
Copy link
Member Author

I've opened #25414 implementing the less radical approach of renaming these functions.

@nalimilan nalimilan closed this Jan 5, 2018
@nalimilan nalimilan deleted the nl/sortedsearch branch January 5, 2018 16:32
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 stdlib Julia's standard library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants