Skip to content

Conversation

topepo
Copy link
Member

@topepo topepo commented Aug 20, 2025

For changes in the important package.

@topepo topepo requested review from EmilHvitfeldt and hfrick August 20, 2025 14:16
Co-authored-by: Emil Hvitfeldt <emilhhvitfeldt@gmail.com>
Copy link
Member

@hfrick hfrick left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dials already has mtry_prop(), predictor_prop(), sample_prop(), and validation_set_prop(). Can you rename this parameter to terms_prop()? I think the more distinguishing part of the name should come first.

@topepo
Copy link
Member Author

topepo commented Aug 21, 2025

It's ordered it that way because it is an analog to the existing arguments in recipes such as num_terms. It's inconsistent with the ones you mentioned, but would be consistent with its direct analogs.

Copy link
Member

@hfrick hfrick left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then I'd like to rename all the num_*() parameters we have 😂

@hfrick hfrick merged commit 005378f into main Aug 22, 2025
15 checks passed
@hfrick hfrick deleted the prop-terms branch August 22, 2025 17:06
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