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

Jump to bottom

Feature 1185 group #3490

Merged
merged 10 commits into from
May 5, 2018
Merged

Feature 1185 group #3490

merged 10 commits into from
May 5, 2018

Conversation

romainfrancois
Copy link
Member

@romainfrancois romainfrancois commented Apr 10, 2018 •
edited by krlmlr
Loading

Closes #3341. Closes #1185.

@krlmlr
Copy link
Member

krlmlr commented Apr 10, 2018

Will the second-to-last commit be gone if you rebase to master?

@krlmlr
Copy link
Member

krlmlr commented Apr 10, 2018

I see that the commit is needed, but maybe you could squash these commits away?

@romainfrancois
Copy link
Member Author

I can squash if you like, but this is not finished yet, see #3491

This was referenced Apr 10, 2018
Closed
Closed
@romainfrancois
give hybrid meaning to group_indices(), as in #1185
bb3fa9d 
```r
> df <- data_frame(v1 = c(NA, NA, 2, 2, 3), v2 = c(NA, NA, 3,3, 4)) %>% group_by(v1)
> df %>% mutate( g = group_indices() )
# A tibble: 5 x 3
# Groups:   v1 [3]
     v1    v2     g
  <dbl> <dbl> <int>
1   NA    NA      3
2   NA    NA      3
3    2.    3.     1
4    2.    3.     1
5    3.    4.     2
```
@romainfrancois
test for #1185. closes #1185
5870af0
@romainfrancois
additional #1185 test for rowwise
fab2fb8
@romainfrancois
group_indices() gives correct result for non grouped df.
1b46290
@romainfrancois
rm FullDataFrame use as in #3487
1bede0b
@romainfrancois
adapt to newer interface
3fdb82b
@romainfrancois
The group method for overloads of SlicingIndex return 0 instead o…
826b41a 
…f -1 for ungrouped data.
@romainfrancois
added group_indices.rowwise_df method.
4e82f54 
test that group_indices actually warns when passed extra arguments.
@romainfrancois
using dots_n instead of length(quos())
83e6306
@romainfrancois
+ NEWS
0925d38
@krlmlr
Copy link
Member

krlmlr commented May 5, 2018

Does this need more work?

@romainfrancois
Copy link
Member Author

It’s good to go as far as i am concerned

@krlmlr krlmlr merged commit dc007a6 into master May 5, 2018
@krlmlr
Copy link
Member

krlmlr commented May 5, 2018

Thanks!

@romainfrancois romainfrancois deleted the feature-1185-group branch May 8, 2018 07:27
@tobadia tobadia mentioned this pull request Jul 24, 2018
Closed
@lock
Copy link

lock bot commented Nov 4, 2018

This old issue has been automatically locked. If you believe you have found a related problem, please file a new issue (with reprex) and link to this issue. https://reprex.tidyverse.org/

@lock lock bot locked and limited conversation to collaborators Nov 4, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@romainfrancois @krlmlr