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

FAQ: why do I need to register methods for S3 generics from other modules/packages? #298

Open
klmr opened this issue Oct 19, 2022 · 3 comments

Comments

@klmr
Copy link
Owner

klmr commented Oct 19, 2022

No description provided.

@klmr
Copy link
Owner Author

klmr commented Aug 27, 2023

Context: Reddit discussion; to wit:

You’ve accurately described ‘box’ semantics here.

No, I accurately described how S3 methods are handled in R.

Right, the issue is that, as you wrote previously (emphasis mine):

You don't need to register S3 methods if the context is local. I.e., if the S3 method is defined in the same environment as it is used.

With modules, S3 methods are not generally defined in the same environment in which they are used (when they are, there’s no issue as you noted). The problem occurs if a user wants to define an S3 method inside a module and use it outside that module. In this case, R won’t find the appropriate method because it only searches the environment in which a generic was called, not the one in which the generic was defined (unless the S3 method is explicitly registered — and, to reiterate ‘box’ does this automatically behind the scenes for the user). — I assume you know this, since you added the relevant code to ‘import’ if I understood you correctly.

@tyner
Copy link

tyner commented Oct 20, 2023

I wonder if this is related to this: ianmcook/implyr#57

@klmr
Copy link
Owner Author

klmr commented Oct 20, 2023

I don’t think it’s related, since S3 classes in packages should just work with ‘box’. And the issue you’re linking to is related to S4 at any rate, not S3.

S4 is another whole can of worms but you should be able to load and use packages that define S4 classes with ‘box’ without issues. Without being an S4 expert, it seems to me that the ’implyr’ package does some questionable things with regards to S4 registration which I believe will cause issues. In particular, it seems to explicitly register S4 classes in the global environment rather than in the package namespace. I don’t think packages are supposed to do that.

I’d love to help diagnose that issue but I don’t have an Impala database handy, and setting up a mock database doesn’t seem trivial without preexisting knowledge. One thing you could try (to see whether this is related to ‘box’) is to perform a manual import of the relevant functions; that is, instead of using box::use(), use the following code:

src_impala <- implyr::src_impala
dbGetQuery <- DBI::dbGetQuery

options(warn=1)
impala <- src_impala(drv = odbc::odbc(),
                     dsn = "my_dsn",
                     database = "default",
                     bigint = "character",
                     auto_disconnect = FALSE
                     )

ret <- dbGetQuery(impala, "show databases")

If this also fails, the error is in ‘implyr’.

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

No branches or pull requests

2 participants