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

Create compatibility with R6 methods? #133

Open
yogat3ch opened this issue Nov 3, 2021 · 8 comments
Open

Create compatibility with R6 methods? #133

yogat3ch opened this issue Nov 3, 2021 · 8 comments

Comments

@yogat3ch
Copy link

yogat3ch commented Nov 3, 2021

It looks like memoise is not compatible with R6 methods. What's the feasibility of creating compatibility with R6?

ther6 <- R6::R6Class(
  "test",
  lock_objects = FALSE,
  public = list(
    fun = memoise::memoise(function() {
      Sys.sleep(5)
      "foo"
    })
    ),
  private = list(
    baz = 1
  )
)

test <- ther6$new()

v <- test$fun()
@wch
Copy link
Member

wch commented Nov 5, 2021

I think the way to make this work is to assign it in the constructor:

ther6 <- R6::R6Class(
  "ther6",
  public = list(
    initialize = function() {
      self$fun <- memoise::memoise(function() {
        message("Running fun()")
        "foo"
      })
    },
    fun = NULL
  )
)

test <- ther6$new()

test$fun()
#> Running fun()
#> [1] "foo"

test$fun()
#> [1] "foo"

Note that fun would be memoized once for each instance of ther6. If you wanted to share the memoized function, you'd have to define it outside of the class, and assign it in the initialize method:

fun_shared <- memoise::memoise(function() {
  message("Running fun()")
  "foo"
})

ther6 <- R6::R6Class(
  "ther6",
  public = list(
    initialize = function() {
      self$fun <- fun_shared
    },
    fun = NULL
  )
)

test <- ther6$new()
test2 <- ther6$new()

test$fun()
#> Running fun()
#> [1] "foo"

test2$fun()
#> [1] "foo"

This obviously won't work well if you want to make use of self in the memoized function.

Another route you could take is to do self$fun <- memoise(....) in the initialize method, as in the first example, but share the caching object across instances.

@yogat3ch
Copy link
Author

yogat3ch commented Jan 6, 2022

I'm definitely using the self object in all the memoised functions. I'll have to try this last method

Another route you could take is to do self$fun <- memoise(....) in the initialize method, as in the first example, but share the caching object across instances.

Thanks for the input!

@teucer
Copy link

teucer commented Feb 1, 2022

@wch Could you please give a concrete implementation for

Another route you could take is to do self$fun <- memoise(....) in the initialize method, as in the first example, but share the caching object across instances.

Thank you!

@wch
Copy link
Member

wch commented Feb 2, 2022

@teucer here you go:

cm <- cachem::cache_mem()

ther6 <- R6::R6Class(
  "ther6",
  public = list(
    initialize = function() {
      self$fun <- memoise::memoise(function() {
        message("Running fun()")
        "foo"
      }, cache = cm)
    },
    fun = NULL
  )
)

test <- ther6$new()

test$fun()
#> Running fun()
#> [1] "foo"

test$fun()
#> [1] "foo"

@teucer
Copy link

teucer commented Feb 2, 2022

@wch this seems to be similar to first version. How is it different?

I think it would advantageous to be able to memoise only once for the whole class (like python class methods).

I tried to initialize fun directly instead of NULL, but it does not work.

@wch
Copy link
Member

wch commented Feb 2, 2022

@teucer Sorry, I copied and pasted the wrong code. I've fixed it in my comment above.

@mmuurr
Copy link

mmuurr commented May 9, 2022

@wch I'm curious if this approach is frowned-upon (when compared to the 'assign-to-NULL' approach shown above):

Klass <- R6::R6Class(
  public = list(
    echo = function(x) {
      print("echoing")
      x
    },
    initialize = function() {
      base::unlockBinding("echo", self)
      self$echo <- memoise::memoise(self$echo)
      base::lockBinding("echo", self)
    }
  )
)

obj <- Klass$new()

obj$echo(1)
# echoing
# [1]

obj$echo(1)
# [1]

The difference here is echo is defined as a function in the class generator object, as opposed to being originally-defined as a NULL field then assigned as a method.

Does this have any practical difference in the final objects? I've done some light testing that suggests, "no", but I might be missing a nuanced detail re: redefining methods vs converting a field to a method. (For example, any concerns with clone()?)

@wch
Copy link
Member

wch commented May 11, 2022

@mmuurr That looks like OK to me. One thing to keep in mind is that, if the function actually uses self, as in self$n, it may be surprising for someone that it won't re-execute when self$n changes. So echo in this case looks like a method, but if you actually do method-y things with it (and access self), you can get into trouble.

If you clone the object, the cloned object's echo function will be the same object as the original's echo, and so self will refer to the wrong thing. Here's an example:

Klass <- R6::R6Class(
  public = list(
    x = 1,
    getx = function() {
      print("echoing")
      self$x
    },
    initialize = function() {
      base::unlockBinding("getx", self)
      self$getx <- memoise::memoise(self$getx)
      base::lockBinding("getx", self)
    }
  )
)

obj <- Klass$new()

obj$getx()
#> [1] "echoing"
#> [1] 1

obj2 <- obj$clone()
obj2$x <- 5
obj2$getx()
#> [1] 1

There may also be problems with inheritance, but I don't know for sure offhand.

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

No branches or pull requests

4 participants