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

group_by_at() doesn't handle existig grouping correctly #3351

Closed
tzoltak opened this issue Feb 9, 2018 · 5 comments
Closed

group_by_at() doesn't handle existig grouping correctly #3351

tzoltak opened this issue Feb 9, 2018 · 5 comments
Assignees
Labels

Comments

@tzoltak
Copy link

@tzoltak tzoltak commented Feb 9, 2018

group_by_at(tbl) is confused if tbl is already a grouped tibble and new grouping uses some of variables that were used in this previously existing grouping. Instead of overwriting this existing grouping (as group_by() does by default) it returns tibble grouped only by variables that wern't used in previosuly defined grouping (and throws a warning about nonexistence of variables common for existing and newly defined grouping).

library(dplyr)
#> 
#> Dołączanie pakietu: 'dplyr'
#> Następujące obiekty zostały zakryte z 'package:stats':
#> 
#>     filter, lag
#> Następujące obiekty zostały zakryte z 'package:base':
#> 
#>     intersect, setdiff, setequal, union

# some grouped tibble
tbl = tibble(gr1 = rep(1:2, 4), gr2 = rep(1:2, each = 4), x = 1:8) %>%
  group_by(gr1)

# doesn't work
tbl %>%
  group_by_at(vars(one_of("gr1", "gr2"))) %>%
  summarise(xSum = sum(x))
#> Warning: Unknown variables: `gr1`

#> Warning: Unknown variables: `gr1`
#> # A tibble: 2 x 2
#>     gr2  xSum
#>   <int> <int>
#> 1     1    10
#> 2     2    26

# adding ungroup() before group_by_at() helps
tbl %>%
  ungroup() %>%
  group_by_at(vars(one_of("gr1", "gr2"))) %>%
  summarise(xSum = sum(x))
#> # A tibble: 4 x 3
#> # Groups:   gr1 [?]
#>     gr1   gr2  xSum
#>   <int> <int> <int>
#> 1     1     1     4
#> 2     1     2    12
#> 3     2     1     6
#> 4     2     2    14

# however group_by() works without such workaround
tbl %>%
  group_by(gr1, gr2) %>%
  summarise(xSum = sum(x))
#> # A tibble: 4 x 3
#> # Groups:   gr1 [?]
#>     gr1   gr2  xSum
#>   <int> <int> <int>
#> 1     1     1     4
#> 2     1     2    12
#> 3     2     1     6
#> 4     2     2    14
@krlmlr
Copy link
Member

@krlmlr krlmlr commented Feb 10, 2018

Thanks, confirmed!

@romainfrancois romainfrancois self-assigned this Mar 12, 2018
@romainfrancois
Copy link
Member

@romainfrancois romainfrancois commented Mar 12, 2018

Same problem as this :

> select_at( tbl, vars(one_of(c("gr1", "gr2"))) )
# A tibble: 8 x 2
# Groups:   gr1 [2]
    gr1   gr2
  <int> <int>
1     1     1
2     2     1
3     1     1
4     2     1
5     1     2
6     2     2
7     1     2
8     2     2
Warning message:
Unknown columns: `gr1` 

@krlmlr
Copy link
Member

@krlmlr krlmlr commented Mar 12, 2018

I just merged a PR from @lionel- that addresses this for select() and rename(), and also updated documentation. Maybe follow the ideas there and extract common logic to functions?

@romainfrancois
Copy link
Member

@romainfrancois romainfrancois commented Mar 12, 2018

Looks like it happens here: https://github.com/tidyverse/dplyr/blob/master/R/colwise.R#L123

where it specifically removes grouping columns.

romainfrancois pushed a commit that referenced this issue Apr 6, 2018
 - group_by_at() can group by an existing grouping variable (#3351).
@lock
Copy link

@lock lock bot commented Oct 3, 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 Oct 3, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants