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

R7 dispatch and S4 inheritance #252

Closed
nbenn opened this issue Sep 12, 2022 · 6 comments · Fixed by #292
Closed

R7 dispatch and S4 inheritance #252

nbenn opened this issue Sep 12, 2022 · 6 comments · Fixed by #292
Labels
bug an unexpected problem or unintended behavior

Comments

@nbenn
Copy link

nbenn commented Sep 12, 2022

Is it not possible for an R7 generic to understand S4 inheritance? Is this an intentional limitation or a bug? (Or am I simply confused for expecting this to work?)

methods::setClass("general")
methods::setClass("specific", contains = "general")

methods::is(methods::new("specific"), "general")
#> [1] TRUE

gen <- methods::getClass("general")

fun <- R7::new_generic("fun", "x")

R7::method(fun, gen) <- function(x) "ok"

fun(methods::new("specific"))
#> Error: Can't find method for generic `fun()` with dispatch classes:
#> - x: S4/specific

(I'm at RConsortium/OOP-WG@879da2a)

@hadley
Copy link
Member

hadley commented Sep 12, 2022

Probably a bug.

@nbenn
Copy link
Author

nbenn commented Sep 12, 2022

Having a closer look at S4_class_dispatch (), my general class is explicitly being excluded from dispatch at

https://github.com/RConsortium/OOP-WG/blob/879da2af57e24243f509e61778a480c9c5dc3ef0/R/S4.R#L81-L83

as it is a "Virtual Class".

@mmaechler
Copy link
Collaborator

Hmm, I've not studied your code nor the details of S4_class_dispatch() but a virtual class in S4 is a class typically meant as a mother class of others or a class to define methods for dispatch.
However there can't be any objects of that S4 class, but just inheriting from that class.
E.g., in the Matrix package there's the virtual class "sparseMatrix" but sparse matrix objects are e.g. of class "dgCMatrix", "lsTMatrix" or ... all inheriting from (or extending) "sparseMatrix".

However, there are many methods defined to dispatch for "sparseMatrix" so maybe there is a bug in S4_class_dispatch() ?

@hadley hadley added the bug an unexpected problem or unintended behavior label Apr 10, 2023
@hadley
Copy link
Member

hadley commented Apr 14, 2023

Updated reprex:

library(methods)
library(S7)

setClass("general")
setClass("specific", contains = "general")

fun <- new_generic("fun", "x")
method(fun, getClass("general")) <- function(x) "general"

fun(new("specific"))
#> Error: Can't find method for generic `fun()` with dispatch classes:
#> - x: S4/specific

Created on 2023-04-14 with reprex v2.0.2

@hadley
Copy link
Member

hadley commented Apr 15, 2023

Or to give a bit more info about what S7 is actually trying:

library(methods)
library(S7)

setClass("general")
setClass("specific", contains = "general")

fun <- new_generic("fun", "x")
method(fun, getClass("general")) <- function(x) "general"

method_explain(fun, object = new("specific"))
#>    fun([S4/specific])
#>    fun([ANY])

Created on 2023-04-15 with reprex v2.0.2

@hadley
Copy link
Member

hadley commented Apr 15, 2023

It looks like S4_class_dispatch() is filtering out virtual classes to avoid dispatching on unions (which are handled elsewhere). It looks like I'll need to use a different method to filter out undesired classes from dispatch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug an unexpected problem or unintended behavior
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants