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

improve error when outcome is missing #1016

Merged
merged 2 commits into from
Nov 3, 2023
Merged

improve error when outcome is missing #1016

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8fd012c
improve error when outcome is missing
simonpcouch Nov 3, 2023
756ec7e
add issue ref to NEWS
simonpcouch Nov 3, 2023
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: 2 additions & 0 deletions NEWS.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
# parsnip (development version)

* Improved errors in cases where the outcome column is mis-specified.
simonpcouch marked this conversation as resolved.
Show resolved Hide resolved

# parsnip 1.1.1

* Fixed bug where prediction on rank deficient `lm()` models produced `.pred_res` instead of `.pred`. (#985)
Expand Down
10 changes: 10 additions & 0 deletions R/misc.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -354,6 +354,16 @@ check_outcome <- function(y, spec) {
return(invisible(NULL))
}

has_no_outcome <- if (is.atomic(y)) {is.null(y)} else {length(y) == 0}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

For is.atomic(y), note is.atomic(NULL) is TRUE, and for length(y) == 0, y can either be a list or tibble.

if (isTRUE(has_no_outcome)) {
cli::cli_abort(
c("!" = "{.fun {class(spec)[1]}} was unable to find an outcome.",
"i" = "Ensure that you have specified an outcome column and that it \\
hasn't been removed in pre-processing."),
call = NULL
simonpcouch marked this conversation as resolved.
Show resolved Hide resolved
)
}

if (spec$mode == "regression") {
outcome_is_numeric <- if (is.atomic(y)) {is.numeric(y)} else {all(map_lgl(y, is.numeric))}
if (!outcome_is_numeric) {
Expand Down
54 changes: 54 additions & 0 deletions tests/testthat/_snaps/misc.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,33 @@
Error in `check_outcome()`:
! For a regression model, the outcome should be `numeric`, not a `factor`.

---

Code
check_outcome(NULL, reg_spec)
Condition
Error:
! `linear_reg()` was unable to find an outcome.
i Ensure that you have specified an outcome column and that it hasn't been removed in pre-processing.

---

Code
check_outcome(tibble::new_tibble(list(), nrow = 10), reg_spec)
Condition
Error:
! `linear_reg()` was unable to find an outcome.
i Ensure that you have specified an outcome column and that it hasn't been removed in pre-processing.

---

Code
fit(reg_spec, ~mpg, mtcars)
Condition
Error:
! `linear_reg()` was unable to find an outcome.
i Ensure that you have specified an outcome column and that it hasn't been removed in pre-processing.

---

Code
Expand All @@ -148,6 +175,33 @@
Error in `check_outcome()`:
! For a classification model, the outcome should be a `factor`, not a `integer`.

---

Code
check_outcome(NULL, class_spec)
Condition
Error:
! `logistic_reg()` was unable to find an outcome.
i Ensure that you have specified an outcome column and that it hasn't been removed in pre-processing.

---

Code
check_outcome(tibble::new_tibble(list(), nrow = 10), class_spec)
Condition
Error:
! `logistic_reg()` was unable to find an outcome.
i Ensure that you have specified an outcome column and that it hasn't been removed in pre-processing.

---

Code
fit(class_spec, ~mpg, mtcars)
Condition
Error:
! `logistic_reg()` was unable to find an outcome.
i Ensure that you have specified an outcome column and that it hasn't been removed in pre-processing.

---

Code
Expand Down
30 changes: 30 additions & 0 deletions tests/testthat/test_misc.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,21 @@ test_that('check_outcome works as expected', {
check_outcome(factor(1:2), reg_spec)
)

expect_snapshot(
error = TRUE,
check_outcome(NULL, reg_spec)
)

expect_snapshot(
error = TRUE,
check_outcome(tibble::new_tibble(list(), nrow = 10), reg_spec)
)

expect_snapshot(
error = TRUE,
fit(reg_spec, ~ mpg, mtcars)
)

class_spec <- logistic_reg()

expect_no_error(
Expand All @@ -220,6 +235,21 @@ test_that('check_outcome works as expected', {
check_outcome(1:2, class_spec)
)

expect_snapshot(
error = TRUE,
check_outcome(NULL, class_spec)
)

expect_snapshot(
error = TRUE,
check_outcome(tibble::new_tibble(list(), nrow = 10), class_spec)
)

expect_snapshot(
error = TRUE,
fit(class_spec, ~ mpg, mtcars)
)

# Fake specification to avoid having to load {censored}
cens_spec <- logistic_reg()
cens_spec$mode <- "censored regression"
Expand Down
Loading