Can't have property named after class in constructor

[jonthegeek]

This one was tricky to find, but this Stack Overflow post about the underlying error message helped me find it.

library(S7)

class_a <- new_class("class_a", properties = list(x = class_character))
class_b <- new_class(
  "class_b",
  properties = list(class_a = class_a),
  constructor = function(class_a = class_a()) {
    # Do things to normalize class_a or whatever.
    new_object(
      S7_object(),
      class_a = class_a
    )
  }
)
class_b()
#> Error in new_object(S7_object(), class_a = class_a): promise already under evaluation: recursive default argument reference or earlier problems?

^{Created on 2023-10-02 with reprex v2.0.2}

The class used as a property specifically has to have the same name as the property to trigger the error. You also need to set class_a() (the constructor call) as the default for class_a (the property).

I think I can work around this (and it might be good to differentiate the instances of the classes (as properties) from the classes, but it led me into a pit of misunderstanding, since supplying class_missing as the default (like in the printed default constructor) seemed to fix things in every case except the thing in #372.

[jonthegeek]

More info: it's specifically the name of the constructor function, not the name of the class:

library(S7)

class_a <- new_class("a", properties = list(x = class_character))
class_b <- new_class(
  "class_b",
  properties = list(class_a = class_a),
  constructor = function(class_a = class_a()) {
    # Do things to normalize class_a or whatever.
    new_object(
      S7_object(),
      class_a = class_a
    )
  }
)
class_b()
#> Error in new_object(S7_object(), class_a = class_a): promise already under evaluation: recursive default argument reference or earlier problems?

^{Created on 2023-10-05 with reprex v2.0.2}

[hadley]

The root cause is almost certainly https://github.com/RConsortium/S7/blob/main/R/constructor.R#L41, where I'm trying to create a default constructor that looks like human written code. It's probably better to instead guarantee that there are no conflicts with user code by adding a non-syntactic prefix.

[jonthegeek]

Does that get called if constructors are supplied? That code seems like a likely culprit, but this still produces the same error:

library(S7)

class_a <- new_class(
  "a",
  properties = list(x = class_character),
  constructor = function(x = character()) {
    new_object(
      S7_object(),
      x = x
    )
  }
)
class_b <- new_class(
  "class_b",
  properties = list(class_a = class_a),
  constructor = function(class_a = class_a()) {
    # Do things to normalize class_a or whatever.
    new_object(
      S7_object(),
      class_a = class_a
    )
  }
)
class_b()
#> Error in new_object(S7_object(), class_a = class_a): promise already under evaluation: recursive default argument reference or earlier problems?

^{Created on 2023-10-05 with reprex v2.0.2}

[hadley]

Well now you've created the problem and are responsible for fixing it, e.g. with:

class_b <- new_class(
  "class_b",
  properties = list(class_a = class_a),
  constructor = function(class_a = NULL) {
    class_a <- class_a %||% class_a()
    # Do things to normalize class_a or whatever.
    new_object(
      S7_object(),
      class_a = class_a
    )
  }
)

But maybe that implies my diagnosis was incorrect and it's always been in your power to fix this?

[hadley]

(I made a note to mention this in the design book: https://github.com/tidyverse/design/issues/172)

[jonthegeek]

The example in the book is helpful! I couldn't wrap my head around what was happening and thought it was S7-specific-ish. With that example, I don't think it is!

Thanks for the clarification! I can deal with this from here. No bug on your end, I think?

[hadley]

Yeah, I think so.