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

count()/add_count() should preserve input classes #4086

Closed
yutannihilation opened this issue Jan 7, 2019 · 7 comments · Fixed by #4751
Closed

count()/add_count() should preserve input classes #4086

yutannihilation opened this issue Jan 7, 2019 · 7 comments · Fixed by #4751
Labels
feature grouping 👨‍👩‍👧‍👦 helpers 🏋️‍♂️ vctrs ↗️
Milestone

Comments

@yutannihilation
Copy link
Member

yutannihilation commented Jan 7, 2019

(Originally reported at #4051 (comment).)

``` r library(dplyr, warn.conflicts = FALSE)

x <- as_tibble(iris)
g <- group_by(x, Species)
class(g) <- c("myclass", class(g))

x %>%
add_tally() %>%
class()
#> [1] "tbl_df" "tbl" "data.frame"

x %>%
add_count() %>%
class()
#> [1] "tbl_df" "tbl" "data.frame"


<sup>Created on 2019-01-07 by the [reprex package](https://reprex.tidyverse.org) (v0.2.1)</sup>
</details>
@romainfrancois romainfrancois added this to the 0.9.0 milestone Jan 23, 2019
@romainfrancois

This comment has been minimized.

@yutannihilation

This comment has been minimized.

@elinw

This comment has been minimized.

@romainfrancois

This comment has been minimized.

@hadley hadley added feature vctrs ↗️ helpers 🏋️‍♂️ labels Dec 11, 2019
@hadley hadley changed the title add_count()/add_tally() strips classes count()/add_count() should preserve input classes Dec 31, 2019
@hadley
Copy link
Member

hadley commented Dec 31, 2019

Minimal reprex:

library(dplyr, warn.conflicts = FALSE)

x <- data.frame(x = 1:3)
x %>% count() %>% class()
#> [1] "tbl_df"     "tbl"        "data.frame"
x %>% add_count() %>% class()
#> [1] "tbl_df"     "tbl"        "data.frame"

Created on 2019-12-31 by the reprex package (v0.3.0)

(tally() and add_tally() don't touch the classes so don't need to be considered further)

@hadley
Copy link
Member

hadley commented Jan 2, 2020

We might want to solve this with a generic mechanism for temporarily grouping, performing some operation, and then restoring groups (some sort of with_groups()), which we could also export.

@hadley
Copy link
Member

hadley commented Jan 8, 2020

Somewhat more comprehensive reprex:

df <- structure(data.frame(x = 1:3), class = c("myclass", "data.frame"))
df %>% count() %>% class()
df %>% count(x) %>% class()
df %>% add_count() %>% class()
df %>% add_count(x) %>% class()

hadley added a commit that referenced this issue Jan 14, 2020
* Start on df_restore()
* count() uses df_restore(); add_count() modifies input
* deprecate .drop argument to `add_count()`

Fixes #4086
hadley added a commit that referenced this issue Jan 15, 2020
This PR starts to develop the dplyr "interface", i.e. the set of generics that you need to provide methods for if you want to extend dplyr to work with new data frame subclasses. It also uses those methods (along with a count_regroups()) to ensure that the existing grouped_df implementations are not needlessly regrouping data.

Fixes #4086 because count() can now use dplyr_reconstruct() to restore the original class
Fixes #4051 because I've carefully documented the return value of the major verbs
Fixes #4711 since implementing with_groups() is now easy.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature grouping 👨‍👩‍👧‍👦 helpers 🏋️‍♂️ vctrs ↗️
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants