Skip to content

Commit

Permalink
address uninformative error for set_engine(NULL) (closes #1199)
Browse files Browse the repository at this point in the history
  • Loading branch information
simonpcouch committed Sep 10, 2024
1 parent 9bb5d0a commit 1ec763a
Show file tree
Hide file tree
Showing 11 changed files with 21 additions and 32 deletions.
2 changes: 1 addition & 1 deletion R/engines.R
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ set_engine.model_spec <- function(object, engine, ...) {
check_spec_mode_engine_val(mod_type, object$engine, object$mode)
}

if (object$engine == "liquidSVM") {
if (!is.null(object$engine) && object$engine == "liquidSVM") {
lifecycle::deprecate_warn(
"0.1.6",
"set_engine(engine = 'cannot be liquidSVM')",
Expand Down
8 changes: 0 additions & 8 deletions tests/testthat/_snaps/nearest_neighbor.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,6 @@
Error in `nearest_neighbor()`:
! "reallyunknown" is not a known mode for model `nearest_neighbor()`.

---

Code
nearest_neighbor() %>% set_engine(NULL)
Condition
Error in `if (object$engine == "liquidSVM") ...`:
! argument is of length zero

# check_args() works

Code
Expand Down
7 changes: 3 additions & 4 deletions tests/testthat/_snaps/rand_forest.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,9 @@
# bad input

Code
translate(rand_forest(mode = "classification") %>% set_engine(NULL))
Condition
Error in `if (object$engine == "liquidSVM") ...`:
! argument is of length zero
res <- translate(rand_forest(mode = "classification") %>% set_engine(NULL))
Message
Used `engine = 'ranger'` for translation.

---

Expand Down
7 changes: 3 additions & 4 deletions tests/testthat/_snaps/surv_reg.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,9 @@
---

Code
translate(surv_reg() %>% set_engine(NULL))
Condition
Error in `if (object$engine == "liquidSVM") ...`:
! argument is of length zero
res <- translate(surv_reg() %>% set_engine(NULL))
Message
Used `engine = 'survival'` for translation.

# deprecation warning

Expand Down
4 changes: 2 additions & 2 deletions tests/testthat/_snaps/svm_linear.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@
Code
translate(svm_linear(mode = "regression") %>% set_engine(NULL))
Condition
Error in `if (object$engine == "liquidSVM") ...`:
! argument is of length zero
Error in `translate.default()`:
! Please set an engine.

---

Expand Down
6 changes: 3 additions & 3 deletions tests/testthat/_snaps/svm_poly.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@
---

Code
svm_poly() %>% set_engine(NULL)
svm_poly() %>% set_engine(NULL) %>% translate()
Condition
Error in `if (object$engine == "liquidSVM") ...`:
! argument is of length zero
Error in `translate.default()`:
! Please set an engine.

4 changes: 2 additions & 2 deletions tests/testthat/_snaps/svm_rbf.md
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,6 @@
Code
translate(svm_rbf(mode = "regression") %>% set_engine(NULL))
Condition
Error in `if (object$engine == "liquidSVM") ...`:
! argument is of length zero
Error in `translate.default()`:
! Please set an engine.

7 changes: 3 additions & 4 deletions tests/testthat/test-nearest_neighbor.R
Original file line number Diff line number Diff line change
Expand Up @@ -8,18 +8,17 @@ test_that('updating', {

test_that('bad input', {
expect_snapshot(error = TRUE, nearest_neighbor(mode = "reallyunknown"))
expect_snapshot(error = TRUE, nearest_neighbor() %>% set_engine( NULL))
})

test_that('check_args() works', {
skip_if_not_installed("kknn")

hpc <- hpc_data[1:150, c(2:5, 8)]

expect_snapshot(
error = TRUE,
{
spec <- nearest_neighbor(neighbors = -1) %>%
spec <- nearest_neighbor(neighbors = -1) %>%
set_engine("kknn") %>%
set_mode("classification")
fit(spec, class ~ ., hpc)
Expand All @@ -28,7 +27,7 @@ test_that('check_args() works', {
expect_snapshot(
error = TRUE,
{
spec <- nearest_neighbor(weight_func = 2) %>%
spec <- nearest_neighbor(weight_func = 2) %>%
set_engine("kknn") %>%
set_mode("classification")
fit(spec, class ~ ., hpc)
Expand Down
4 changes: 2 additions & 2 deletions tests/testthat/test-rand_forest.R
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ test_that('updating', {
})

test_that('bad input', {
expect_snapshot(error = TRUE, translate(rand_forest(mode = "classification") %>% set_engine(NULL)))
expect_snapshot(res <- translate(rand_forest(mode = "classification") %>% set_engine(NULL)))
expect_snapshot(error = TRUE, rand_forest(mode = "time series"))
expect_snapshot(error = TRUE, translate(rand_forest(mode = "classification") %>% set_engine("wat?")))
expect_snapshot(error = TRUE, translate(rand_forest(mode = "classification", ytest = 2)))
Expand All @@ -17,4 +17,4 @@ test_that('bad input', {
test_that("check_args() works", {
# Here for completeness, no checking is done
expect_true(TRUE)
})
})
2 changes: 1 addition & 1 deletion tests/testthat/test-surv_reg.R
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ test_that('bad input', {

expect_snapshot(error = TRUE, surv_reg(mode = ", classification"))
expect_snapshot(error = TRUE, translate(surv_reg() %>% set_engine("wat")))
expect_snapshot(error = TRUE, translate(surv_reg() %>% set_engine(NULL)))
expect_snapshot(res <- translate(surv_reg() %>% set_engine(NULL)))
})

test_that("deprecation warning", {
Expand Down
2 changes: 1 addition & 1 deletion tests/testthat/test-svm_poly.R
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ test_that('updating', {

test_that('bad input', {
expect_snapshot(error = TRUE, svm_poly(mode = "reallyunknown"))
expect_snapshot(error = TRUE, svm_poly() %>% set_engine(NULL))
expect_snapshot(error = TRUE, svm_poly() %>% set_engine(NULL) %>% translate())
})

# ------------------------------------------------------------------------------
Expand Down

0 comments on commit 1ec763a

Please sign in to comment.