-
Notifications
You must be signed in to change notification settings - Fork 38
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Can't have property named after class in constructor #373
Comments
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 |
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. |
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 |
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? |
(I made a note to mention this in the design book: tidyverse/design#172) |
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? |
Yeah, I think so. |
This one was tricky to find, but this Stack Overflow post about the underlying error message helped me find it.
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 forclass_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.The text was updated successfully, but these errors were encountered: