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

Add ... to signature of db_list_tables #5610

Closed
wants to merge 2 commits into from

Conversation

MichaelChirico
Copy link
Contributor

@MichaelChirico MichaelChirico commented Nov 18, 2020

We have an extension which needs further arguments. I'll file another PR so as not to break dbplyr shortly.

Actually I see that as of tidyverse/dbplyr#499, dbplyr no longer uses this generic. Will it be deprecated here as well?

MichaelChirico and others added 2 commits November 18, 2020 11:15
We have an extension which needs further arguments. I'll file another PR so as not to break `dbplyr` shortly.
@hadley
Copy link
Member

hadley commented Nov 20, 2020

@MichaelChirico the deprecation plan is described in https://dbplyr.tidyverse.org/articles/backend-2.html. I think you probably just want to modify the dbplyr generic.

@MichaelChirico
Copy link
Contributor Author

Thanks... closing here. Will come back to this later.

@MichaelChirico
Copy link
Contributor Author

@hadley looking at this again, I'm a bit confused. Did you mean to say to modify the dplyr generic?

db_list_tables isn't a part of dbplyr anymore from what I'm seeing at HEAD...

@hadley
Copy link
Member

hadley commented Nov 30, 2020

Sorry, I forgot which generics went where — instead of db_list_tables, dbplyr now uses DBI::dbListTables(). But it looks like that was only ever used as part of db_write_table() (https://dbplyr.tidyverse.org/articles/backend-2.html#unused-generics-1) so you might actually want to define a dbWriteTable() method. What are you trying to do?

@MichaelChirico
Copy link
Contributor Author

We have a method that takes another argument to db_list_tables, but the generic doesn't have ..., so I was just going to add that to facilitate extensions.

We use db_list_tables directly for interactive cases, so dbWriteTable feels... indirect. Any thoughts?

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

Successfully merging this pull request may close these issues.

None yet

2 participants