Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

add_count() always calls dplyr_reconstruct() #5846

Closed
wants to merge 1 commit into from

Conversation

romainfrancois
Copy link
Member

closes #5837

This probably needs dplyr_reconstruct() dispatch to be simplified:

dplyr_reconstruct <- function(data, template) {
  # Strip attributes before dispatch to make it easier to implement
  # methods and prevent unexpected leaking of irrelevant attributes.
  data <- dplyr_new_data_frame(data)
  return(dplyr_reconstruct_dispatch(data, template))
  UseMethod("dplyr_reconstruct", template)
}

so that it can be implemented for tbl_lazy in dbplyr, or perhaps dplyr_new_data_frame() should work for tbl_lazy:

library(dplyr, warn.conflicts = FALSE)
db <- dbplyr::memdb_frame(x = c(1, 1, 1, 2, 2))
dplyr:::dplyr_new_data_frame(db)
#> Error: Input must be a vector, not a <src_SQLiteConnection/src_dbi/src_sql/src> object.

Created on 2021-04-09 by the reprex package (v0.3.0)

@romainfrancois romainfrancois requested a review from hadley April 9, 2021 09:41
@hadley
Copy link
Member

hadley commented Apr 9, 2021

I think it would better to make add_count() generic.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Should add_count() be generic?
2 participants