Skip to content

Conversation

@sostock
Copy link
Contributor

@sostock sostock commented Jul 29, 2020

Currently, there is no specialized unique implementation for ranges, so unique falls back to the default implementation for iterators, which seems wasteful.

This PR implements unique(::AbstractRange) and simplifies allunique(::AbstractRange).

unique(::AbstractRange) now returns a range instead of a Vector. Is it okay that unique does not return a mutable collection, or should I change it to return a Vector like before?

@StefanKarpinski StefanKarpinski added the minor change Marginal behavior change acceptable for a minor release label Jul 29, 2020
@StefanKarpinski
Copy link
Member

I think this is ok to change in a minor release. This does seem like a good optimization to have.

@StefanKarpinski StefanKarpinski added the triage This should be discussed on a triage call label Jul 29, 2020
@sostock
Copy link
Contributor Author

sostock commented Jul 30, 2020

The allunique simplification is actually a bugfix (cf. #36849), so I will split this PR into two, one containing the allunique bugfix and one containing the new unique method.

@sostock sostock closed this Jul 30, 2020
@StefanKarpinski StefanKarpinski removed the triage This should be discussed on a triage call label Oct 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

minor change Marginal behavior change acceptable for a minor release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants