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

unique and duplicated's default value for 'by=' #1284

Closed
arunsrinivasan opened this issue Aug 20, 2015 · 5 comments
Closed

unique and duplicated's default value for 'by=' #1284

arunsrinivasan opened this issue Aug 20, 2015 · 5 comments
Milestone

Comments

@arunsrinivasan
Copy link
Member

I am not particularly happy with the current option of unique/duplicated having by = key(x) as the default, especially because there's no message or warning as to which columns are being used to compute unique values on.

Now that on= argument is also in place, and we are entertaining the idea of having to set keys less, maybe it's right time to lift this default value so that it's similar to base R? i.e., with cols = seq_along(x) - see #1283.

@franknarf1
Copy link
Contributor

I agree.

(I'm guessing a "question" issue is soliciting opinions..? I've no opinion on the by->cols issue; I like by, but could live with cols. I have never written the cols arg of the other functions out, rather passing by position every time. In contrast, I always write out by=.)

@arunsrinivasan
Copy link
Member Author

Meant it to be internals, sorry about that.

@MichaelChirico
Copy link
Member

I agree--I rarely use the default value for by in my current usage--even if x is keyed, more often I do something equivalent to by=key(x)[1:2], i.e., only using a subset of the assigned keys.

@arunsrinivasan arunsrinivasan added this to the v1.9.8 milestone Sep 22, 2015
@arunsrinivasan arunsrinivasan modified the milestones: v2.0.0, v1.9.8 Apr 10, 2016
@jangorecki
Copy link
Member

that would also refer to uniqueN

@cderv
Copy link

cderv commented Nov 29, 2016

Help documentation for these functions seems not to have changed. It still says that by = key(x) is used by default. I think we should update the documentation as it is not verry clear right now.

I discovered this reading through the NEWS for all changes in 1.9.8.

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

No branches or pull requests

6 participants