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

adapt test on interaction w/ parsnip re intercept #214

Merged
merged 2 commits into from
Dec 13, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion DESCRIPTION
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ Imports:
hardhat (>= 1.2.0),
lifecycle (>= 1.0.3),
modelenv (>= 0.1.0),
parsnip (>= 1.1.0.9001),
parsnip (>= 1.1.1.9004),
rlang (>= 1.0.3),
tidyselect (>= 1.2.0),
vctrs (>= 0.4.1)
Expand Down
2 changes: 1 addition & 1 deletion tests/testthat/_snaps/pre-action-recipe.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
Error in `add_recipe()`:
! A recipe cannot be added when variables already exist.

# cannot add a recipe if recipes is trained
# cannot add a recipe if recipe is trained
hfrick marked this conversation as resolved.
Show resolved Hide resolved

Code
add_recipe(workflow, rec)
Expand Down
15 changes: 8 additions & 7 deletions tests/testthat/test-predict.R
Original file line number Diff line number Diff line change
Expand Up @@ -134,20 +134,21 @@ test_that("blueprint will get passed on to hardhat::forge()", {
)
})

test_that("monitoring: known that parsnip removes blueprint intercept for some models (tidymodels/parsnip#353)", {
test_that("monitoring: no double intercept due to dot expansion in model formula #210", {
mod <- parsnip::linear_reg()
mod <- parsnip::set_engine(mod, "lm")

# Pass formula explicitly to keep `lm()` from auto-generating an intercept
# model formula includes a dot to mean "everything available after the preprocesing formula
hfrick marked this conversation as resolved.
Show resolved Hide resolved
workflow <- workflow()
workflow <- add_model(workflow, mod, formula = mpg ~ . + 0)
workflow <- add_model(workflow, mod, formula = mpg ~ .)

blueprint_with_intercept <- hardhat::default_formula_blueprint(intercept = TRUE)
workflow_with_intercept <- add_formula(workflow, mpg ~ hp + disp, blueprint = blueprint_with_intercept)
fit_with_intercept <- fit(workflow_with_intercept, mtcars)

# `parsnip:::prepare_data()` will remove the intercept, so it won't be
# there when the `lm()` `predict()` method is called. We don't own this
# error though.
expect_error(predict(fit_with_intercept, mtcars))
# The dot expansion used to include the intercept column, added via the blueprint, as a regular predictor.
# `parsnip:::prepare_data()` removed this column, so lm's predict method errored.
# Now it get removed before fitting (lm will handle the intercept itself),
hfrick marked this conversation as resolved.
Show resolved Hide resolved
# so lm()'s predict method won't error anymore here.
hfrick marked this conversation as resolved.
Show resolved Hide resolved
expect_no_error(predict(fit_with_intercept, mtcars))
})
Loading