Add unique!(f, itr) #28415


#1

Should unique!(f, itr) support Real,AbstractString and Symbol just as unique!(itr) do?
Also do we have to take care of the order of occurence when implementing it?


#2

Hi,

unique! needs to make sure that it only works on types that can actually be updated in place (in this case: AbstractVector). e.g. unique!(1:3) won’t work, because you can’t update a range in place.

There is a fast-path implementation available for unique!(itr::AbstractVector{...}) if the element types of the AbstractVector can be sorted, which is the case for Real, AbstractString and Symbol, but not automatically for other element types.
The same fast-path implementation doesn’t work for unique!(f, itr), as you would only now if its sorted after applying f, which might be quite costly and you wouldn’t want to execute it twice. (with a type unstable f you wouldn’t even know if all elements can be sorted…)

I had an implementation that mixed the issorted-fastpath checking with calling f while performing the unqiue operation and therefore circumventing the cost of calling f twice, but the added complexity wasn’t worth the improvement.

I’ve made a PR a while ago which improves situation and makes the various function consistent in their implementation (e.g. automatic widening):


#3

I have created a PR RFC : Added naive unique!(f, itr) function #30141 which fixes #28415.
Please review it and comment for further amendments in it.

Earlier, I accidentally pushed it directly to origin/master branch for which I don’t have permission.
Please tell me if it would cause any further problems?


#4

I think the PR has been amended to the best of my knowledge and thanks to @oxinabox for helping me out.I am not getting why travis-ci build is failing repeatedly, any advice would be helpful.


#5

Have you benchmarked it against my solution in the linked PR? Especially if you regard all kinds of element set your PR might not be optimal.


#6

I will benchmark it against your solution.Thanks for guiding me.


#7

When I run @btime unique!(x->2x, 1:10_000); it says setindex! not defined for UnitRange{Int64}. I couldn’t understand what the error message says?
Same problem occurs with the @benchmark macro.