RConsortium / S7

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

Abstract class validators are not inherited by child objects #329

Closed jl5000 closed 1 year ago

jl5000 commented 1 year ago

Even though you cannot create abstract classes, it's still very convenient to define validators within them to save having to define them for every child class.

class_test = S7::new_class("class_test",
                           properties = list(x = S7::class_double),
                           validator = function(self){
                             if(self@x >= 3) return("Error: should be less than 3")
                           })

class_test_child <- S7::new_class("class_test_child", parent = class_test)

class_test_child(1)
#> <class_test_child>
#>  @ x: num 1
class_test_child(4)
#> Error: <class_test> object is invalid:
#> - Error: should be less than 3

class_test = S7::new_class("class_test", abstract = TRUE,
                           properties = list(x = S7::class_double),
                           validator = function(self){
                             if(self@x >= 3) return("Error: should be less than 3")
                           })

class_test_child <- S7::new_class("class_test_child", parent = class_test)

class_test_child(1)
#> <class_test_child>
#>  @ x: num 1
class_test_child(4)
#> <class_test_child>
#>  @ x: num 4

Created on 2023-08-29 with reprex v2.0.2

mmaechler commented 1 year ago

I agree. This has been a too narrow restriction for abstract classes. Almost surely a patch would be welcome.

hadley commented 1 year ago

This looks like a fairly subtle bug — I think it's because constructors normally call validate() with recursive = FALSE because you expect the parent constructor to have done any validation it needs, but obviously we don't use the parent constructor when it is abstract. So I suspect we need a tweak to https://github.com/RConsortium/S7/blob/main/R/class.R#L261