Skip to content

Use check_outcome for all fit paths #625

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 2 commits into from
Dec 15, 2021
Merged

Use check_outcome for all fit paths #625

merged 2 commits into from
Dec 15, 2021

Conversation

juliasilge
Copy link
Member

Closes #621

This PR updates form_form() and form_xy() so that all four fit paths use the same check_outcome() helper function.

parsnip::fit(
  parsnip::decision_tree(mode = "regression"),
  status ~ wind,
  data = dplyr::storms)
#> Error in `check_outcome()` at parsnip/R/fit_helpers.R:9:4:
#> For a regression model, the outcome should be numeric.

Created on 2021-12-14 by the reprex package (v2.0.1)

@juliasilge juliasilge requested a review from hfrick December 14, 2021 19:11
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.

Looks good! The test you added runs xy_form() and there are some which use form_form(). Do we also have one for form_xy()? If not, we could add one with glmnet.

library(parsnip)

linear_reg(penalty = 0.1) %>%
  set_engine("glmnet") %>%
  fit(status ~ wind, data = dplyr::storms)
#> Error: For a regression model, the outcome should be numeric.

Created on 2021-12-15 by the reprex package (v2.0.1)

@juliasilge
Copy link
Member Author

That would have to go into extratests because no glmnet tests get executed here in parsnip. I guess the pass through the fitting is different enough that it's worth doing that.

@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 Dec 30, 2021
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Regression models should error when outcome is character
2 participants