Skip to content

uniquify the docstring of unique(itr) #22368

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

Jump to bottom
Merged
merged 1 commit into from
Jun 18, 2017
Merged

uniquify the docstring of unique(itr) #22368

merged 1 commit into from
Jun 18, 2017

Conversation

rfourquet
Copy link
Member

The docs for unique(itr) seems to have been redundant, cf. #15741 (comment).

@rfourquet rfourquet added the docs This change adds or pertains to documentation label Jun 14, 2017
ararslan
ararslan reviewed Jun 14, 2017
View reviewed changes
base/multidimensional.jl Outdated
@@ -1298,9 +1298,16 @@ hash(x::Prehashed) = x.hash

Returns an array containing only the unique elements of the iterable `itr`, in
the order that the first of each set of equivalent elements originally appears.
The unicity is determined by [`isequal`](@ref).
Copy link
Member

Choose a reason for hiding this comment

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

I think "unicity" is a fairly uncommon word. (Indeed, Firefox spellcheck is marking it as incorrect, though a Google search shows it exists.) I think "uniqueness" might be clearer here.

Copy link
Member Author

Choose a reason for hiding this comment

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

For reference to those interested: https://english.stackexchange.com/questions/225094/uniqueness-vs-unicity

Copy link
Member

Choose a reason for hiding this comment

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

I gotta say, I do love the suggestions in one of the answers regarding rhyming. We really need more rhyming docstrings.

@nalimilan
Copy link
Member

See #20800, which I should have merged a few weeks ago.

@rfourquet
Copy link
Member Author

@nalimilan oups I hadn't search for open PR on that, sorry. I will check if there is something left of this one now that you merged.

@rfourquet rfourquet force-pushed the rf/unique-unique-doc branch from 21ad159 to e292400 Compare June 17, 2017 10:22
@rfourquet
Copy link
Member Author

So I still think the docstring were redundant, I updated accordingly. I also updated the example unique([1; 2; 2; 6]) into unique([1, 2, 6, 2]) to make it clear that the duplicates don't have to be grouped for unique to uniquify them.

nalimilan
nalimilan approved these changes Jun 18, 2017
View reviewed changes
Copy link
Member

@nalimilan nalimilan left a comment

Choose a reason for hiding this comment

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

Indeed, my PR didn't get rid of repetitions.

@ararslan ararslan merged commit bd2878f into master Jun 18, 2017
@ararslan ararslan deleted the rf/unique-unique-doc branch June 18, 2017 19:40
quinnj pushed a commit that referenced this pull request Jun 20, 2017
@rfourquet @quinnj
uniquify the docstring of unique(itr) (#22368)
9249d93
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ararslan ararslan ararslan left review comments

@nalimilan nalimilan nalimilan approved these changes

@Sacha0 Sacha0 Sacha0 approved these changes

Assignees
No one assigned
Labels
docs This change adds or pertains to documentation
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@rfourquet @nalimilan @Sacha0 @ararslan