-
Notifications
You must be signed in to change notification settings - Fork 38
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 does not seem to work with class unions #72
Comments
Reprex: library(R7)
bar <- new_generic(name = "bar", signature = c("x", "y"))
setClassUnion("char_OR_NULL", c("character", "NULL"))
method(bar, list("char_OR_NULL", "numeric")) <- function(x, y, ...) paste0("foo-", x, ":", y)
bar("foo", 1)
#> Error: No method found!
bar(NULL, 1)
#> Error: No method found! Created on 2021-10-22 by the reprex package (v2.0.1) |
It does work with R7 unions FWIW, e.g. library(R7)
bar <- new_generic(name = "bar", signature = c("x", "y"))
chr_or_null <- new_union("character", "NULL")
method(bar, chr_or_null) <- function(x, y, ...) paste0("foo-", x, ":", y)
bar("foo", 1)
#> [1] "foo-foo:1"
bar(NULL, 1)
#> [1] "foo-:1" Created on 2021-10-26 by the reprex package (v2.0.1) I am not sure it should work with S4 class unions? |
If (e.g.) Bioc wanted to shift from S4 to R7, I suspect it would be useful to be able to switch generics while leaving the class structure as is. Should |
I agree it would be nice and possibly important to have "glue functionality" providing compatibility. If this (partial S4 support) seems a bit unclean, might it be a possibilty to have the functionality "modularized" in an extra S4toR7 or R7forS4 package which would depend on methods and R7 ? |
Revisit after #134 |
With #134 library(R7)
char_OR_NULL <- setClassUnion("char_OR_NULL", c("character", "NULL"))
foo <- new_generic("foo", dispatch_args = "x")
method(foo, char_OR_NULL) <- function(x) "char_or_NULL"
foo("foo", 1)
#> Error: Can't find method for generic `foo()` with classes:
#> - x: <character>
foo(NULL, 1)
#> Error: Can't find method for generic `foo()` with classes:
#> - x: <NULL> Created on 2022-01-26 by the reprex package (v2.0.1) I think the easiest way to deal with this will be to convert S4 unions to R7 unions, so the same method dispatch logic can apply. Does this seem like the right way to get the classes that an S4 union applies to? Foo <- setClass("Foo", slots = "x")
setClassUnion("optionalFoo", c("Foo", "NULL"))
union <- getClass("optionalFoo")
subclasses <- Filter(function(x) x@distance == 1, union@subclasses)
lapply(subclasses, function(x) getClass(x@subClass))
#> $`NULL`
#> Class "NULL" [package "methods"]
#>
#> No Slots, prototype of class "NULL"
#>
#> Extends: "OptionalFunction", "optionalMethod", "optionalFoo"
#>
#> Known Subclasses:
#> Class ".NULL", directly, with explicit coerce
#>
#> $Foo
#> Class "Foo" [in ".GlobalEnv"]
#>
#> Slots:
#>
#> Name: x
#> Class: ANY
#>
#> Extends: "optionalFoo" Created on 2022-01-26 by the reprex package (v2.0.1) Translating S4 unions to R7 unions also means that we need to be able to translate the S4 wrappers of base types to the R7 wrappers of base types, i.e. we need to be able to translate |
The overall approach sounds good to me. For finding the members of the union, it may be better to recurse instead in order to handle nested unions. |
Now library(R7)
char_OR_NULL <- setClassUnion("char_OR_NULL", c("character", "NULL"))
foo <- new_generic("foo", dispatch_args = "x")
method(foo, char_OR_NULL) <- function(x) "char_or_NULL"
foo("foo")
#> [1] "char_or_NULL"
foo(NULL)
#> [1] "char_or_NULL" Created on 2022-02-08 by the reprex package (v2.0.1) |
The text was updated successfully, but these errors were encountered: