r-lib / memoise

Easy memoisation for R
https://memoise.r-lib.org
Other
317 stars 56 forks source link

Compatibility with S4 Methods (feature request/bug) #147

Open KellenBrosnahan opened 1 year ago

KellenBrosnahan commented 1 year ago

Hello, I am currently writing a package using the S4 OOP system, and was wondering whether there was an existing canonical way to memoize S4 methods. One of the errors I got was that setMethod cannot be passed a memoized function because the S3 class memoised is not technically an S4 class extending function, which is what the MethodDefinition class (for S4 methods) requires:

Error in initialize(value, ...) : 
  cannot use object of class “memoised” in new():  class “MethodDefinition” does not extend that class

A simple workaround is to formally declare memoised to be an S4 class extending function via the following code:

setClass("memoised", contains = "function", slots = character(), prototype = new("function"))

Is there a reason not to do this? Should this be part of the memoise package (assuming methods is loaded)?

Here's a reprex:

f <- function(x){x}
memF <- memoise::memoise(f)
setClass("TestClass", slots = character(), prototype = list())
setGeneric("testMethod", function(x) standardGeneric("testMethod"))
setMethod("testMethod", signature = "TestClass", memF)
wch commented 1 year ago

Sounds reasonable to me (but I am also not an S4 expert).

@hadley What do you think?

hadley commented 1 year ago

If we were to do that, we'd use setOldClass(), but that sort of S4 manipulation can have unexpected and far reaching complications, so I wouldn't recommend it.