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

RSQLite::dbWriteTable can't handle grouped_df #2276

Closed
joranE opened this issue Nov 30, 2016 · 8 comments
Closed

RSQLite::dbWriteTable can't handle grouped_df #2276

joranE opened this issue Nov 30, 2016 · 8 comments

Comments

@joranE
Copy link
Contributor

@joranE joranE commented Nov 30, 2016

I'm filing this here, since it seems specific to the case of a tbl_df with the S3 class signature of:

[1] "grouped_df" "tbl_df"     "tbl"        "data.frame"

and it seems like the issue was raised in RSQLite, but eventually solved for the case of just

[1] "tbl_df"     "tbl"        "data.frame"

in tibble. Let me know if you'd prefer it filed somewhere else...

Here's the setup:

library(DBI)
library(RSQLite)
library(dplyr)
mtcars <- group_by(.data = mtcars,cyl)

d> class(mtcars)
[1] "grouped_df" "tbl_df"     "tbl"        "data.frame"

mydb <- dbConnect(RSQLite::SQLite(), ":memory:")

d> dbWriteTable(conn = mydb,name = "mtcars",value = mtcars)
Error in (function (classes, fdef, mtable)  : 
  unable to find an inherited method for function ‘dbWriteTable’ for signature ‘"SQLiteConnection", "character", "grouped_df"’

Based on what I read in previous issues, I guess a suitable setOldClass() directive needs to be issued somewhere in dplyr for grouped_df's.

Session Info:

d> sessionInfo()
R version 3.3.2 (2016-10-31)
Platform: x86_64-apple-darwin13.4.0 (64-bit)
Running under: OS X El Capitan 10.11.5

locale:
[1] en_US.UTF-8/en_US.UTF-8/en_US.UTF-8/C/en_US.UTF-8/en_US.UTF-8

attached base packages:
[1] stats     graphics  grDevices utils     datasets  methods   base     

other attached packages:
[1] DBI_0.5-12      dplyr_0.5.0     RSQLite_1.1     devtools_1.12.0

loaded via a namespace (and not attached):
 [1] lazyeval_0.2.0 magrittr_1.0.1 R6_2.1.2       assertthat_0.1 tools_3.3.2    withr_1.0.2    tibble_1.2    
 [8] Rcpp_0.12.7    memoise_1.0.0  digest_0.6.9  
@krlmlr
Copy link
Member

@krlmlr krlmlr commented Dec 1, 2016

@hadley: Do you agree we need setOldClass(c("grouped_df", "tbl_df", "data.frame")) ? Are there any drawbacks?

@hadley
Copy link
Member

@hadley hadley commented Dec 1, 2016

I'm pretty sure that's correct. I don't see any downsides.

Do we need to get in the habit of doing this whenever we create a new S3 class?

@krlmlr
Copy link
Member

@krlmlr krlmlr commented Dec 1, 2016

@hadley: Surely if we need S4 compatibility, but this perhaps doesn't apply for the vector S3 classes such as hms and blob?

@joranE: Would you like to contribute a pull request?

@hadley
Copy link
Member

@hadley hadley commented Dec 1, 2016

Yeah, we don't normally need S4 compatibility, but should we always do it as a courtesy for the S4 users who might want to?

@krlmlr
Copy link
Member

@krlmlr krlmlr commented Dec 1, 2016

In the case of dbWriteTable() we could make it a nonstandard generic that calls as.data.frame() on the input, but I wonder if this is really safe.

@hadley
Copy link
Member

@hadley hadley commented Dec 1, 2016

Yeah, I think that's a bad idea because it means that it's dbWriteTable is more likely to not give an error when you give it seriously wrong input.

@joranE
Copy link
Contributor Author

@joranE joranE commented Dec 1, 2016

@krlmlr Sure; I presume it just amounts to adding setOldClass(c("grouped_df", "tbl_df", "data.frame")) as the first line to dataframe.R?

@hadley
Copy link
Member

@hadley hadley commented Jan 31, 2017

@joranE I'd put it immediately after the grouped_df constructor function

@hadley hadley closed this in f89bea1 Feb 16, 2017
@lock lock bot locked as resolved and limited conversation to collaborators Jun 8, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants