Skip to content

Accept initializer in new_class() and new_property()#448

Closed
t-kalinowski wants to merge 1 commit intoinclude-dynamic-settable-props-in-constructorfrom
class-and-prop-init
Closed

Accept initializer in new_class() and new_property()#448
t-kalinowski wants to merge 1 commit intoinclude-dynamic-settable-props-in-constructorfrom
class-and-prop-init

Conversation

@t-kalinowski
Copy link
Member

This PR is a sketch of one possible solution for allowing users some more control over what property setters get called at object construction without too much hassle (i.e., without requiring a custom constructor.)

#445 (comment)

Now that I look at this, it doesn't seem quite right either.

Perhaps a new_property(..., virtual = TRUE) argument, which would mean the property is not set at construction and is not included as a constructor argument.

@hadley
Copy link
Member

hadley commented Sep 24, 2024

Yeah, maybe a new argument to new_property() is the way to go.

@t-kalinowski t-kalinowski marked this pull request as draft September 24, 2024 18:32
@lawremi
Copy link
Collaborator

lawremi commented Sep 24, 2024

I am not sure how removing something from the constructor solves the problem of the deprecated property, because it still needs to be settable for backwards compatibility.

@t-kalinowski
Copy link
Member Author

Given the example use case of a deprecated property, the goal is to not warn if the user didn't supply an explicit value in the constructor but warn if they did and also warn in all other cases of the property being set. Decoupling setting at initialization from setting all other times might be one approach towards making that possible.

However, for this example, I think there is a more minimal approach using ..., in #445. (We may want to revisit explicit initializers in the future, but that would be a separate issue.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants