RConsortium / S7

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

Disallow `NULL` as a `.parent` in `new_object()` #345

Closed DavisVaughan closed 1 year ago

DavisVaughan commented 1 year ago

I think this tightens things up a bit. It is technically breaking as NULL was allowed before. I think we should either do this or set .parent = S7_object() as the default (which would parallel parent = S7_object as the default in new_class()).

If we did .parent = S7_object(), then I'd be ok with this optional argument coming before the named ... in this instance because if you have a .parent that takes arguments, then those arguments would be supplied before the class arguments when the constructor is generated, so you end up with something like:

# good
my_class <- function(x, y) {
  new_object(parent(x = x), y = y)
}

# harder to read
my_class <- function(x, y) {
  new_object(y = y, .parent = parent(x = x))
}
jonthegeek commented 1 year ago

Was it allowed? I don't remember the exact series of events right now, but, in testing out #337, I generated an error because something was trying to add attributes to NULL.

I'm getting

Error: Unsupported `parent` type

when I try

library(S7)
my_class <- new_class(
  "my_class",
  parent = NULL
)

Edit: Oops, this is in new_object(), gotcha.