RConsortium / S7

S7: a new OO system for R
https://rconsortium.github.io/S7
Other
386 stars 32 forks source link

class_missing vs method dispatch #372

Closed jonthegeek closed 10 months ago

jonthegeek commented 10 months ago

I expected this to work:

# pak::pkg_install("RConsortium/S7") # Use latest dev build.
library(S7)

my_fun <- new_generic("my_fun", dispatch_args = "x")
method(my_fun, class_missing | NULL) <- function(x) {
  "In the method."
}

my_fun()
#> [1] "In the method."
my_fun(NULL)
#> [1] "In the method."
my_fun(class_missing)
#> Error: Can't find method for `my_fun(S3<S7_missing>)`.

Created on 2023-10-02 with reprex v2.0.2

Context: I have as_class_a() functions for class_a that go beyond its constructor, and I wanted to use them in the constructor of class_b (which has class_a as a property). I was surprised to see that calling them without an argument triggers an error, rather than being handled by the missing/NULL method.

Adding new_S3_class("S7_missing") to the method's dispatch union makes it work as expected:

method(my_fun, class_missing | NULL | new_S3_class("S7_missing")) <- function(x) {
  "In the method."
}

my_fun(class_missing)
#> [1] "In the method."

Created on 2023-10-02 with reprex v2.0.2

hadley commented 10 months ago

You don't supply class_missing to the call; you just leave the argument missing:

library(S7)

my_fun <- new_generic("my_fun", dispatch_args = "x")
method(my_fun, class_missing | NULL) <- function(x) {
  "In the method."
}

my_fun()
#> [1] "In the method."
my_fun(NULL)
#> [1] "In the method."
my_fun()
#> [1] "In the method."

Created on 2023-10-02 with reprex v2.0.2

hadley commented 10 months ago

Or is this a duplicate of #356?

jonthegeek commented 10 months ago

I think it's probably related to #356. Actually that's almost definitely it; I thought I was supposed to supply class_missing like in the default constructor, but I'm guessing this will go away if I build my constructor more like a normal function (like I now see in the Constructors section of the vignette).

Let me try some experiments, one moment.

jonthegeek commented 10 months ago

Ok, I was doing that because of a different issue... which I'm having trouble tracking down for a reprex. Hopefully either I'll squash it or open a new issue shortly. I think this particularly issue can be considered a duplicate/confusion related to #356.