Skip to content

Commit

Permalink
improve error when outcome is missing (#1016)
Browse files Browse the repository at this point in the history
  • Loading branch information
simonpcouch authored Nov 3, 2023
1 parent 1afcd2f commit b462f76
Show file tree
Hide file tree
Showing 4 changed files with 96 additions and 0 deletions.
2 changes: 2 additions & 0 deletions NEWS.md
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. (#1003)

# 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
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}
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
)
}

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
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
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

0 comments on commit b462f76

Please sign in to comment.