underlying data corrupt

[sims1253]

While trying to get something to work, I got to this minimal example, which resulted in an error that sounded like something in the S7 internals broke. Maybe I did someting obviously wrong, but I figured I'd file an issue just in case:

Error: <TestClass> object is invalid:
- Underlying data is corrupt
4.
stop(msg, call. = FALSE)
3.
function (object, recursive = TRUE, properties = TRUE)
{
check_is_S7(object)
if (!is.null(attr(object, ".should_validate"))) { ...
2.
`prop<-`(`*tmp*`, "x", value = 10)
1.
TestClass(x = 10)

Minimal Example:

TestClass <- S7::new_class(
    "TestClass",
    properties = list(
        x = S7::new_property(default = NULL, validator = function(value) {
            if (!is.null(value) && !is.numeric(value)) {
                "@x must be numeric"
            }
        })
    ),
    constructor = function(x = NULL) {
        obj <- new_object(.parent = TestClass)
        if (!is.null(x)) {
            prop(obj, "x") <- x
        }
        obj
    }
)

test_obj <- TestClass(x = 10)

[hadley]

Hmmmm, the problem is that you're trying to work with the object before it's finished construction. I'm not sure if we could make a better error message here, but it would be nice if possible.

[sims1253]

I suppose that would be a more informative error message :) It sounds a lot less like something internal is broken.

In case you're interested how I ended up here: This started with me trying to use (...) in the constructor which didn't work as do.call can't call new_object and I don't know another way of of calling a function with a list of unknown arguments.
Then I figured I'd just NULL all the properties that haven't been passed, as is.null felt clearer than if(length>0) for all the numeric()'s it created for empty properties instead. Which I didn't get to work the way I wanted and ultimately ended up as this example while trying to figure out what I was doing wrong.

[sims1253]

Curious, is this the same problem?

library(S7)

SimpleClass <- S7::new_class('SimpleClass',
  properties = list(
    name = class_character
  ),
  constructor = function() {
    new_object(SimpleClass, name = "dog")
  }
)

obj <- SimpleClass()  # This works
obj@name              # This works
obj@name <- "cat"     # This fails with "Underlying data is corrupt"

[lawremi]

Perhaps the man page is confusing here, because it describes both new_class() and new_object(). new_class() takes an argument parent=, which is the parent class object, while new_object() takes the argument .parent= (note the preceding dot), which should be an instance of the parent class. Besides adding some guards against this in new_object(), we should probably separate the two functions into their own man page, as it breeds confusion with little benefit. Finally, if possible, it's best to avoid overriding the constructor of a class, since it is really easy to make mistakes like these.

[JosiahParry]

Here's a reprex. I am only encountering this when using a constructor. When I don't use a constructor this is fine.

library(S7)
Ulid <- new_property(
  class_character,
  validator = function(value) {
    if (!is.na(value)) {
      stopifnot(
        "must be a scalar character" = length(value) == 1,
        "ulids are 26 characters" = nchar(value) == 26
      )
    }
  },
  default = ulid::ulid()
)

Todo <- S7::new_class(
  "Todo",
  properties = list(
    id = Ulid,
    category = Ulid
  ),
)

todo <- Todo(category = ulid::ulid())
todo@id <- ulid::ulid()

constructor <- function(category) {
  S7::new_object(
    Todo,
    id = ulid::ulid(),
    category = category
  )
}

Todo <- S7::new_class(
  "Todo",
  constructor = constructor,
  properties = list(
    id = Ulid,
    category = Ulid
  ),
)

todo <- Todo(category = ulid::ulid())
todo@id <- ulid::ulid()
#> Error: <Todo> object is invalid:
#> - Underlying data is corrupt