r-lib / R6

Encapsulated object-oriented programming for R
https://R6.r-lib.org
Other
404 stars 56 forks source link

Update method in base class and pass to inherited classes #154

Closed admercs closed 5 years ago

admercs commented 5 years ago

Hi,

I'm getting a strange issue in that when I override a NULL R6 class method inherited from a base class and the new method contains references to self, I receive the error object 'self' not found. However, self is clearly referenced here. The weird thing is that if I try to directly override the function or use the Class$set() method with overwrite=TRUE the instantiated object still does not contain the method.

A related issue is that I would like to override a method in a base class and push the change to classes that inherit from base.

gaborcsardi commented 5 years ago

Please show a reproducible example.

A related issue is that I would like to override a method in a base class and push the change to classes that inherit from base.

That will not work with R6, by design. Maybe you can somehow work around it by putting the "updated" (?) method to a place where the subclass objects can look it up.

admercs commented 5 years ago

Sorry, the classes are very large and cannot be shared here. The essence of the problem is that I cannot override a NULL method in either a base or inherited class unless I use a set_method() helper function. It appears to overwrite correctly, but the instantiated object does not contain the method. Also, if I used overwrite=TRUE with Class$set() on the base class I receive a Can't add method_name because it already present in Class generator error. Using the set_method() helper function throws the object 'self' not found error when I try to use the overridden method. I get the same error if I instead use public or super. Nothing seems to work.

This is frustrating unexpected behaviour. Any idea what might cause it? Otherwise, I have loved R6 classes so far.

admercs commented 5 years ago

Here is a toy example showing the problem:

Parent <- R6::R6Class("Parent",
  public = list(
    values    = list(x=list(), y=list()),
    get_value = NULL,
    set_value = function(x_values, get_method) {
      self$values$x  = x_values
      self$get_value = get_method
      message("Values and getter method set")
    }
  )
)

Child <- R6::R6Class("Child",
  inherit = Parent,
  public = list(
    value = 1
  )
)

x <- list(a=1, b=2, c=3)
get_x <- function(...) {
  return(self$values$x)
}

obj <- Child$new()
obj$set_value(x_values=x, get_method=get_x)
obj$get_value()
Error in obj$get_value() : object 'self' not found
gaborcsardi commented 5 years ago

This will not work, because get_x_sum() does not have the right environment. I don't think there is a way to add a method from the instance, i.e. from a method.

wch commented 5 years ago

Echoing @gaborcsardi: the issue isn't with inheritance, but trying to assign a function as a method. Methods of R6 objects have a particular environment where they can find self. This example illustrates:

Parent <- R6::R6Class("Parent",
  public = list(
    value = 1,
    get_value = NULL,
    set_value = function(x, get_method) {
      self$value     <- x
      self$get_value <- get_method
    }
  )
)

get_self_value <- function(...) {
  self$value
}

obj <- Parent$new()
obj$set_value(2, get_self_value)
obj$get_value()
#> Error in obj$get_value() : object 'self' not found

In this case, obj$get_value can find y from the environment where the get_self_value function was defined, but it can't find self.

It is possible to modify the environment of the passed-in function so that it behaves like a proper method. However, this is a hack, and I'm not sure if this will work correctly when inheritance is involved:

Parent <- R6::R6Class("Parent",
  public = list(
    value = 1,
    get_value = NULL,
    set_value = function(x, get_method) {
      self$value <- x
      # Modify environment of get_method
      environment(get_method) <- parent.env(environment())
      self$get_value <- get_method
    }
  )
)

get_self_value <- function(...) {
  self$value
}

obj <- Parent$new()
obj$set_value(2, get_self_value)
obj$get_value()
#> [1] 2

Another, better workaround is to pass in a function that takes self as an argument, and save that in a private field. The get_value method is simply a wrapper that calls the passed-in function:

Parent <- R6::R6Class("Parent",
  public = list(
    value = 1,
    get_value = function(...) {
      private$get_value_(self, ...)
    },
    set_value = function(x, get_method) {
      self$value <- x
      private$get_value_ <- get_method
    }
  ),
  private = list(
    get_value_ = NULL
  )
)

get_self_value <- function(self, ...) {
  self$value
}

obj <- Parent$new()
obj$set_value(2, get_self_value)
obj$get_value()
#> [1] 2

This is the best non-hacky solution that I can think of.

If you do this, note that there will be an issue with environments if the object is cloned: https://github.com/r-lib/R6/issues/94#issuecomment-245930469. I am planning on fixing that issue, though.

admercs commented 5 years ago

Thank you, @wch! Just for completeness, here are the other two methods that do not work the way one might expect:

Child$values$x  <- x
Child$get_value <- get_x
obj <- Child$new()
obj$get_value()
Error: attempt to apply non-function
Child$set("public", "values$x", x, overwrite=TRUE)
Child$set("public", "get_value", get_x, overwrite=TRUE)
obj <- Child$new()
obj$get_value()
Error: attempt to apply non-function

And here is what happens when setting the list member:

obj$values$x
list()
obj$`values$x`
$a
[1] 1

$b
[1] 2

$c
[1] 3

Running the same tests on Parent gives the same results plus the additional weird error:

Parent$set("public", "get_value", get_x, overwrite=TRUE)
Error in Parent$set("public", "get_value", get_x, overwrite = TRUE) : 
  Can't add get_value because it already present in Parent generator.
admercs commented 5 years ago

Finally, the method environment hack worked best for me. Much simpler than the alternative and it works with inheritance.