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.

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

Allow other preprocessors #9

Merged
merged 8 commits into from
Jun 1, 2022
Merged

Allow other preprocessors #9

merged 8 commits into from
Jun 1, 2022

Conversation

qiushiyan
Copy link
Collaborator

@qiushiyan qiushiyan commented May 27, 2022

This PR should allow tuning when there is no recipe and other types of preprocessors #5.Not sure if we need to change this line https://github.com/topepo/agua/blob/e789ffa750262ffb31c46c50c41e755e9a8cd9d5/R/tune.R#L27 seems to work when there is no recipe.

We also need additional internal tune functions forge_from_workflow and finalize_workflow_preprocessor

collections
}

bind_metrics_iter_grid <- function(metrics, iter_grid) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

bind .config to metrics

# prep training and validation data
training_frame_processed <- recipes::bake(preprocessor, new_data = training_frame)
val_frame_processed <- recipes::bake(preprocessor, new_data = val_frame)
training_forged <- tune:::forge_from_workflow(training_frame,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

use forge_from_workflow instead of recipes::bake, works when there is no recipe

@qiushiyan qiushiyan changed the title Allow other preprocessors #5 Allow other preprocessors May 27, 2022
@qiushiyan qiushiyan requested a review from topepo May 27, 2022 20:40
@topepo
Copy link
Member

topepo commented May 28, 2022

That that line is fine.

I think that it's time to add tests (with recipes, formulas, and variable specifications). They'll have to skip for now but we can run them locally.

@qiushiyan qiushiyan merged commit 7722a6c into main Jun 1, 2022
@qiushiyan qiushiyan deleted the preprocessor branch June 1, 2022 15:20
@github-actions
Copy link

This pull request has been automatically locked. If you believe you have found a related problem, please file a new issue (with a reprex: https://reprex.tidyverse.org) and link to this issue.

@github-actions github-actions bot locked and limited conversation to collaborators Jan 10, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants