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 workflows with formulas #59

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

Conversation

topepo
Copy link

@topepo topepo commented Jan 29, 2024

Closes #56

The main API change is that a fitted workflow would now be required. That made the changes a little more invasive than I hoped they would be since it affects documentation and testing.

I could make the testing code a little more simple with some changes that would avoid a bunch of expect_type() and expect_s3_class() calls. Now sure how much more you would like me to make more changes.

On the bright side:

library(tidymodels)
library(workboots)
car_subset <- mtcars[, c("mpg", "disp", "wt")]
lm_wflow <- workflow(mpg ~ ., parsnip::linear_reg())
lm_fit <- fit(lm_wflow, car_subset)
new_car <- data.frame(disp = 150.0, wt = 2.5)
set.seed(1)
new_car_pred <-
  predict_boots(
    workflow = lm_fit,
    n = 2000,
    training_data = car_subset,
    new_data = new_car
  ) %>% 
  summarise_predictions()

new_car_pred
#> # A tibble: 1 × 5
#>   rowid .preds               .pred .pred_lower .pred_upper
#>   <int> <list>               <dbl>       <dbl>       <dbl>
#> 1     1 <tibble [2,000 × 2]>  23.9        17.7        29.8

Created on 2024-01-29 with reprex v2.0.2

@@ -0,0 +1,83 @@
# ---
# repo: tidymodels/workflows
Copy link
Author

Choose a reason for hiding this comment

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

This "standalone" file is in the next version of workflows and is designed to be added to other packages. It is tested there and the nocov directives prevent it from decreasing code coverage here.

@@ -1,19 +1,17 @@
# read in data to use in tests
Copy link
Author

@topepo topepo Jan 29, 2024

Choose a reason for hiding this comment

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

I moved these to a "helper.R" file. Since it starts with "helper" it gets executed prior to each test file.

)

# predictors missing from new_data
expect_error(
expect_snapshot(
Copy link
Author

Choose a reason for hiding this comment

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

I moved most of the expect_error() code to expect_snapshot() since it tends to be more robust to unrelated changes to the package.

)

# tests
expect_s3_class(x, c("tbl_df", "tbl", "data.frame"))
Copy link
Author

Choose a reason for hiding this comment

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

We could reduce a lot of these by making a "ptype" list which is just a zero-row slice of the data (e.g. x[0,]). That contains the classes of the overall object as well as the columns.

@@ -235,7 +237,7 @@ ames_boot_conf_int %>%
geom_point(aes(y = Sale_Price),
alpha = 0.25) +
scale_x_log10(labels = scales::comma_format()) +
scale_y_log10(labels = scales::label_number(scale_cut = scales::cut_short_scale())) +
Copy link
Author

Choose a reason for hiding this comment

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

These were causing an error. I, for the life of me, could not find a way to resolve it. I might be able to ask Thomas about it.

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.

Allow workflows without recipes
1 participant