Skip to content
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.

Sign up for GitHub

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

Jump to bottom

revisiting tunable() for model specifications #1105

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

revisiting tunable() for model specifications #1105

wants to merge 3 commits into from

Conversation

simonpcouch
Copy link
Contributor

@simonpcouch simonpcouch commented Apr 5, 2024 •
edited
Loading

Going ahead and PRing now to introduce tests and run GHA with them so I can demonstrate they still pass once I make some upcoming changes.

EDIT: Closes #1104.

@simonpcouch simonpcouch linked an issue Apr 5, 2024 that may be closed by this pull request
tunable.model_spec() includes rows for non-tunable arguments marked with tune() #1104
Open
simonpcouch added a commit to tidymodels/extratests that referenced this pull request Apr 5, 2024
@simonpcouch
remove skipped tests
00ca1b8 
instead of re-enabling as in #215, test where functionality is defined in tidymodels/parsnip#1105
@simonpcouch
Copy link
Contributor Author

simonpcouch commented Apr 5, 2024 •
edited
Loading

extract_parameter_set_dials() tests fail because they expect anything marked with tune() to appear in tunable() output.

Next steps would be to:

  • Fill out the component_id() column in tune_args() instead of filling with NAs, and
  • Redefine the join as a left join in extract_parameter_set_dials.model_spec() and look for the component_id in tune_args().

If those changes restore previous extract_parameter_set_dials.model_spec() behavior, the next step would be to revise and test tunable() methods in parsnip extensions. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

tunable.model_spec() includes rows for non-tunable arguments marked with tune()
1 participant
@simonpcouch