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.

Sign up for GitHub

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

[WIP] Adding unique! functionality #20576

Closed
wants to merge 8 commits into from
Closed

[WIP] Adding unique! functionality #20576

Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
efa33b7
Adding `unique!` functionality
kvmanohar22 Feb 11, 2017
e973e8b
modified the method a bit
kvmanohar22 Feb 12, 2017
97b4797
More efficient method to delete duplicate indices
kvmanohar22 Feb 12, 2017
905e877
Adding doc examples and small changes
kvmanohar22 Feb 13, 2017
892552b
Adding test cases
kvmanohar22 Feb 14, 2017
95e4c19
update tests
kvmanohar22 Feb 14, 2017
1183329
remove trailing whitespace
kvmanohar22 Feb 14, 2017
9ff810c
adding unique! to exports
kvmanohar22 Feb 15, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions base/set.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,23 @@ function unique(f::Callable, C)
out
end

"""
unique!(itr)
Copy link
Member

Choose a reason for hiding this comment

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

Removing elements is generally not possible on iterators, so better replace itr with c (for collection).

Copy link
Member

@ararslan ararslan Feb 14, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

The signature has been changed, but not the signature in the docstring. The docstring should also be updated.


Removes recurrences of items and returns the modified collection
Copy link
Member

Choose a reason for hiding this comment

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

Use imperative "Remove" and add ending dot. Also, better say "duplicated items" rather than "recurrence". Finally, would be worth saying a word about the fact that isequal is used (like for unique), and adding one or two examples (see how to do so in the Documentation section of the manual).

"""
function unique!(itr)
seen = Set{eltype(itr)}()
Copy link
Member

Choose a reason for hiding this comment

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

Please use 4 spaces rather than tabs for indentation for consistency with the rest of the repo.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should have noted that, will update it in the next commit

for (i, x) in enumerate(itr)
if x in seen
deleteat!(itr, i)
else
push!(seen, x)
end
end
return itr
end

"""
allunique(itr) -> Bool

Expand Down