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: patched searchsortedfirst, searchsortedlast, searchsorted, and insorted available in DataStructures.jl? #782

Open
StephenVavasis opened this issue Dec 23, 2021 · 3 comments

Comments

@StephenVavasis
Copy link
Contributor

A number of Julia users including me have been stung by unexpected behavior in the searchsorted suite of functions in Base, namely, that the by transformation is applied also to the key. There is a recent discussion (https://discourse.julialang.org/t/problem-with-searchsortedfirst/73332/31) of this on discourse, but it has come up before. This makes the suite awkward to use when the by function is something like first that extracts part of a larger object. A PR JuliaLang/julia#35418 to fix this problem was rejected by the core team. I am wondering: Would it make sense to offer patched versions of these Base function in the DataStructures package? Note DataStructures already overloads Base.searchsortedfirst, etc., specifically for sorted containers, so it would not be totally random.

@oxinabox
Copy link
Member

No, I don't think so?
Not if that means monkey-patching Base via type-piracy.
It's too evil to inflict on our >1000 downstream packages.

@StephenVavasis
Copy link
Contributor Author

StephenVavasis commented Jan 11, 2022

I was thinking of a new type, say SortedVector, that would patch this issue. So the user would say: searchsorted(SortedVector(my_vector, extract_key_from_record=(x->x.first)), key, by=func). Then the docstring for Base.searchsorted could briefly mention the awkwardness of applying by to the key and the existence of this alternative.

@oxinabox
Copy link
Member

ah, in that case i think it is less evil, but better to put in a new package in JuliaCollections or elsewhere.
I think you should have the rights sufficient to create such a new package

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants