From 6f7435373f9408cb069f13d6adabc4048f3ed225 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E2=80=98topepo=E2=80=99?= Date: Wed, 29 Jan 2025 11:51:26 -0500 Subject: [PATCH 1/5] changes to #1242 --- R/engines.R | 2 +- tests/testthat/test-engines.R | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-) create mode 100644 tests/testthat/test-engines.R diff --git a/R/engines.R b/R/engines.R index fceea94eb..1eb2ec9bd 100644 --- a/R/engines.R +++ b/R/engines.R @@ -111,7 +111,7 @@ set_engine <- function(object, engine, ...) { set_engine.model_spec <- function(object, engine, ...) { mod_type <- class(object)[1] - if (rlang::is_missing(engine)) { + if (rlang::is_missing(engine) | is.null(engine)) { stop_missing_engine(mod_type, call = caller_env(0)) } object$engine <- engine diff --git a/tests/testthat/test-engines.R b/tests/testthat/test-engines.R new file mode 100644 index 000000000..68b612c52 --- /dev/null +++ b/tests/testthat/test-engines.R @@ -0,0 +1,8 @@ +test_that("NULL engines", { + # See issue https://github.com/tidymodels/parsnip/issues/1242 + + expect_snapshot( + set_engine(nearest_neighbor(), NULL), + error = TRUE + ) +}) From c6798e4b8d38a30286287a5123bb0a20443793af Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E2=80=98topepo=E2=80=99?= Date: Sat, 1 Feb 2025 08:32:40 -0500 Subject: [PATCH 2/5] updated test cases --- NEWS.md | 2 ++ tests/testthat/_snaps/args_and_modes.md | 8 ++++---- tests/testthat/_snaps/engines.md | 8 ++++++++ tests/testthat/_snaps/mars.md | 4 ++-- tests/testthat/_snaps/multinom_reg.md | 4 ++-- tests/testthat/_snaps/nullmodel.md | 4 ++-- tests/testthat/_snaps/rand_forest.md | 5 +++-- tests/testthat/_snaps/surv_reg.md | 5 +++-- tests/testthat/_snaps/svm_linear.md | 4 ++-- tests/testthat/_snaps/svm_poly.md | 4 ++-- tests/testthat/_snaps/svm_rbf.md | 4 ++-- tests/testthat/test-rand_forest.R | 5 ++++- tests/testthat/test-surv_reg.R | 2 +- 13 files changed, 37 insertions(+), 22 deletions(-) create mode 100644 tests/testthat/_snaps/engines.md diff --git a/NEWS.md b/NEWS.md index d184da56a..c8f6bccc0 100644 --- a/NEWS.md +++ b/NEWS.md @@ -41,6 +41,8 @@ * The quantile regression prediction type was disabled for the deprecated `surv_reg()` model. +* `NULL` is no longer accepted as an engine (#1242). + # parsnip 1.2.1 diff --git a/tests/testthat/_snaps/args_and_modes.md b/tests/testthat/_snaps/args_and_modes.md index af1ee702b..175f926da 100644 --- a/tests/testthat/_snaps/args_and_modes.md +++ b/tests/testthat/_snaps/args_and_modes.md @@ -108,16 +108,16 @@ Code linear_reg() %>% set_engine() Condition - Error in `set_engine()`: - ! Missing engine. Possible mode/engine combinations are: quantile regression {quantreg} and regression {lm, glm, glmnet, stan, spark, keras, brulee}. + Error in `set_engine.model_spec()`: + ! argument "engine" is missing, with no default --- Code proportional_hazards() %>% set_engine() Condition - Error in `set_engine()`: - ! No known engines for `proportional_hazards()`. + Error in `set_engine.model_spec()`: + ! argument "engine" is missing, with no default # set_* functions error when input isn't model_spec diff --git a/tests/testthat/_snaps/engines.md b/tests/testthat/_snaps/engines.md new file mode 100644 index 000000000..dd1342c9c --- /dev/null +++ b/tests/testthat/_snaps/engines.md @@ -0,0 +1,8 @@ +# NULL engines + + Code + set_engine(nearest_neighbor(), NULL) + Condition + Error in `set_engine()`: + ! Missing engine. Possible mode/engine combinations are: classification {kknn} and regression {kknn}. + diff --git a/tests/testthat/_snaps/mars.md b/tests/testthat/_snaps/mars.md index 75ef9ba90..5e5d9c4ce 100644 --- a/tests/testthat/_snaps/mars.md +++ b/tests/testthat/_snaps/mars.md @@ -19,8 +19,8 @@ Code translate(mars(mode = "regression") %>% set_engine()) Condition - Error in `set_engine()`: - ! Missing engine. Possible mode/engine combinations are: classification {earth} and regression {earth}. + Error in `set_engine.model_spec()`: + ! argument "engine" is missing, with no default --- diff --git a/tests/testthat/_snaps/multinom_reg.md b/tests/testthat/_snaps/multinom_reg.md index 78d2ef3ad..89d053fd7 100644 --- a/tests/testthat/_snaps/multinom_reg.md +++ b/tests/testthat/_snaps/multinom_reg.md @@ -37,8 +37,8 @@ Code multinom_reg(penalty = 0.1) %>% set_engine() Condition - Error in `set_engine()`: - ! Missing engine. Possible mode/engine combinations are: classification {glmnet, spark, keras, nnet, brulee}. + Error in `set_engine.model_spec()`: + ! argument "engine" is missing, with no default # check_args() works diff --git a/tests/testthat/_snaps/nullmodel.md b/tests/testthat/_snaps/nullmodel.md index 79ac405b1..3465197b2 100644 --- a/tests/testthat/_snaps/nullmodel.md +++ b/tests/testthat/_snaps/nullmodel.md @@ -3,8 +3,8 @@ Code translate(null_model(mode = "regression") %>% set_engine()) Condition - Error in `set_engine()`: - ! Missing engine. Possible mode/engine combinations are: classification {parsnip} and regression {parsnip}. + Error in `set_engine.model_spec()`: + ! argument "engine" is missing, with no default --- diff --git a/tests/testthat/_snaps/rand_forest.md b/tests/testthat/_snaps/rand_forest.md index fc233adc6..7265dcc6a 100644 --- a/tests/testthat/_snaps/rand_forest.md +++ b/tests/testthat/_snaps/rand_forest.md @@ -19,8 +19,9 @@ Code res <- translate(rand_forest(mode = "classification") %>% set_engine(NULL)) - Message - Used `engine = 'ranger'` for translation. + Condition + Error in `set_engine()`: + ! Missing engine. Possible mode/engine combinations are: classification {ranger, randomForest, spark} and regression {ranger, randomForest, spark}. --- diff --git a/tests/testthat/_snaps/surv_reg.md b/tests/testthat/_snaps/surv_reg.md index 66039a420..9a2ae9610 100644 --- a/tests/testthat/_snaps/surv_reg.md +++ b/tests/testthat/_snaps/surv_reg.md @@ -32,8 +32,9 @@ Code res <- translate(surv_reg() %>% set_engine(NULL)) - Message - Used `engine = 'survival'` for translation. + Condition + Error in `set_engine()`: + ! Missing engine. Possible mode/engine combinations are: regression {flexsurv, survival}. # deprecation warning diff --git a/tests/testthat/_snaps/svm_linear.md b/tests/testthat/_snaps/svm_linear.md index 10f05e864..7edcaf122 100644 --- a/tests/testthat/_snaps/svm_linear.md +++ b/tests/testthat/_snaps/svm_linear.md @@ -20,8 +20,8 @@ Code translate(svm_linear(mode = "regression") %>% set_engine(NULL)) Condition - Error in `translate.default()`: - ! Please set an engine. + Error in `set_engine()`: + ! Missing engine. Possible mode/engine combinations are: classification {LiblineaR, kernlab} and regression {LiblineaR, kernlab}. --- diff --git a/tests/testthat/_snaps/svm_poly.md b/tests/testthat/_snaps/svm_poly.md index be1508036..b2430e8c2 100644 --- a/tests/testthat/_snaps/svm_poly.md +++ b/tests/testthat/_snaps/svm_poly.md @@ -28,6 +28,6 @@ Code svm_poly() %>% set_engine(NULL) %>% translate() Condition - Error in `translate.default()`: - ! Please set an engine. + Error in `set_engine()`: + ! Missing engine. Possible mode/engine combinations are: classification {kernlab} and regression {kernlab}. diff --git a/tests/testthat/_snaps/svm_rbf.md b/tests/testthat/_snaps/svm_rbf.md index d7649eb3b..6ad5c97ac 100644 --- a/tests/testthat/_snaps/svm_rbf.md +++ b/tests/testthat/_snaps/svm_rbf.md @@ -48,6 +48,6 @@ Code translate(svm_rbf(mode = "regression") %>% set_engine(NULL)) Condition - Error in `translate.default()`: - ! Please set an engine. + Error in `set_engine()`: + ! Missing engine. Possible mode/engine combinations are: classification {kernlab, liquidSVM} and regression {kernlab, liquidSVM}. diff --git a/tests/testthat/test-rand_forest.R b/tests/testthat/test-rand_forest.R index 242b14cba..647b06908 100644 --- a/tests/testthat/test-rand_forest.R +++ b/tests/testthat/test-rand_forest.R @@ -8,7 +8,10 @@ test_that('updating', { }) test_that('bad input', { - expect_snapshot(res <- translate(rand_forest(mode = "classification") %>% set_engine(NULL))) + expect_snapshot(res <- + translate(rand_forest(mode = "classification") %>% + set_engine(NULL)), + error = TRUE) expect_snapshot(error = TRUE, rand_forest(mode = "time series")) expect_snapshot(error = TRUE, translate(rand_forest(mode = "classification") %>% set_engine("wat?"))) }) diff --git a/tests/testthat/test-surv_reg.R b/tests/testthat/test-surv_reg.R index 8a0d34582..366a8e898 100644 --- a/tests/testthat/test-surv_reg.R +++ b/tests/testthat/test-surv_reg.R @@ -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(res <- translate(surv_reg() %>% set_engine(NULL))) + expect_snapshot(res <- translate(surv_reg() %>% set_engine(NULL)), error = TRUE) }) test_that("deprecation warning", { From 46b57de11b8e6cd47562dbda0a3dd285158529e8 Mon Sep 17 00:00:00 2001 From: Max Kuhn Date: Mon, 10 Feb 2025 16:20:53 -0500 Subject: [PATCH 3/5] Update R/engines.R Co-authored-by: Simon P. Couch --- R/engines.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R/engines.R b/R/engines.R index 1eb2ec9bd..86e3ffd87 100644 --- a/R/engines.R +++ b/R/engines.R @@ -111,7 +111,7 @@ set_engine <- function(object, engine, ...) { set_engine.model_spec <- function(object, engine, ...) { mod_type <- class(object)[1] - if (rlang::is_missing(engine) | is.null(engine)) { + if (rlang::is_missing(engine) || is.null(engine)) { stop_missing_engine(mod_type, call = caller_env(0)) } object$engine <- engine From 00bb1b88ffbfc9938703cdf4998b69fe00bd3345 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E2=80=98topepo=E2=80=99?= Date: Tue, 11 Feb 2025 15:17:03 -0500 Subject: [PATCH 4/5] rerun tests and redoc --- man/details_linear_reg_glmer.Rd | 2 +- man/details_linear_reg_lmer.Rd | 2 +- man/details_linear_reg_stan_glmer.Rd | 2 +- man/details_logistic_reg_glmer.Rd | 2 +- man/details_logistic_reg_stan_glmer.Rd | 2 +- man/details_poisson_reg_glmer.Rd | 2 +- man/details_poisson_reg_stan_glmer.Rd | 2 +- tests/testthat/_snaps/args_and_modes.md | 8 ++++---- tests/testthat/_snaps/mars.md | 4 ++-- tests/testthat/_snaps/mlp_keras.md | 10 ++++++++++ tests/testthat/_snaps/multinom_reg.md | 4 ++-- tests/testthat/_snaps/nullmodel.md | 4 ++-- 12 files changed, 27 insertions(+), 17 deletions(-) diff --git a/man/details_linear_reg_glmer.Rd b/man/details_linear_reg_glmer.Rd index 67d8745bb..d4da1c9a4 100644 --- a/man/details_linear_reg_glmer.Rd +++ b/man/details_linear_reg_glmer.Rd @@ -52,7 +52,7 @@ linear predictor (\verb{\eta}) for a random intercept: \if{html}{\out{
}}\preformatted{\eta_\{i\} = (\beta_0 + b_\{0i\}) + \beta_1x_\{i1\} }\if{html}{\out{
}} -where $i$ denotes the \code{i}th independent experimental unit +where \code{i} denotes the \code{i}th independent experimental unit (e.g. subject). When the model has seen subject \code{i}, it can use that subject’s data to adjust the \emph{population} intercept to be more specific to that subjects results. diff --git a/man/details_linear_reg_lmer.Rd b/man/details_linear_reg_lmer.Rd index 0441f464a..4e4b5a34d 100644 --- a/man/details_linear_reg_lmer.Rd +++ b/man/details_linear_reg_lmer.Rd @@ -44,7 +44,7 @@ linear predictor (\verb{\eta}) for a random intercept: \if{html}{\out{
}}\preformatted{\eta_\{i\} = (\beta_0 + b_\{0i\}) + \beta_1x_\{i1\} }\if{html}{\out{
}} -where $i$ denotes the \code{i}th independent experimental unit +where \code{i} denotes the \code{i}th independent experimental unit (e.g. subject). When the model has seen subject \code{i}, it can use that subject’s data to adjust the \emph{population} intercept to be more specific to that subjects results. diff --git a/man/details_linear_reg_stan_glmer.Rd b/man/details_linear_reg_stan_glmer.Rd index 3bcb67ddf..78132f4bb 100644 --- a/man/details_linear_reg_stan_glmer.Rd +++ b/man/details_linear_reg_stan_glmer.Rd @@ -64,7 +64,7 @@ linear predictor (\verb{\eta}) for a random intercept: \if{html}{\out{
}}\preformatted{\eta_\{i\} = (\beta_0 + b_\{0i\}) + \beta_1x_\{i1\} }\if{html}{\out{
}} -where $i$ denotes the \code{i}th independent experimental unit +where \code{i} denotes the \code{i}th independent experimental unit (e.g. subject). When the model has seen subject \code{i}, it can use that subject’s data to adjust the \emph{population} intercept to be more specific to that subjects results. diff --git a/man/details_logistic_reg_glmer.Rd b/man/details_logistic_reg_glmer.Rd index b848df19c..a87a9e9c6 100644 --- a/man/details_logistic_reg_glmer.Rd +++ b/man/details_logistic_reg_glmer.Rd @@ -44,7 +44,7 @@ linear predictor (\verb{\eta}) for a random intercept: \if{html}{\out{
}}\preformatted{\eta_\{i\} = (\beta_0 + b_\{0i\}) + \beta_1x_\{i1\} }\if{html}{\out{
}} -where $i$ denotes the \code{i}th independent experimental unit +where \code{i} denotes the \code{i}th independent experimental unit (e.g. subject). When the model has seen subject \code{i}, it can use that subject’s data to adjust the \emph{population} intercept to be more specific to that subjects results. diff --git a/man/details_logistic_reg_stan_glmer.Rd b/man/details_logistic_reg_stan_glmer.Rd index ce1281501..628702e45 100644 --- a/man/details_logistic_reg_stan_glmer.Rd +++ b/man/details_logistic_reg_stan_glmer.Rd @@ -63,7 +63,7 @@ linear predictor (\verb{\eta}) for a random intercept: \if{html}{\out{
}}\preformatted{\eta_\{i\} = (\beta_0 + b_\{0i\}) + \beta_1x_\{i1\} }\if{html}{\out{
}} -where $i$ denotes the \code{i}th independent experimental unit +where \code{i} denotes the \code{i}th independent experimental unit (e.g. subject). When the model has seen subject \code{i}, it can use that subject’s data to adjust the \emph{population} intercept to be more specific to that subjects results. diff --git a/man/details_poisson_reg_glmer.Rd b/man/details_poisson_reg_glmer.Rd index 5a32c17bd..520798925 100644 --- a/man/details_poisson_reg_glmer.Rd +++ b/man/details_poisson_reg_glmer.Rd @@ -44,7 +44,7 @@ linear predictor (\verb{\eta}) for a random intercept: \if{html}{\out{
}}\preformatted{\eta_\{i\} = (\beta_0 + b_\{0i\}) + \beta_1x_\{i1\} }\if{html}{\out{
}} -where $i$ denotes the \code{i}th independent experimental unit +where \code{i} denotes the \code{i}th independent experimental unit (e.g. subject). When the model has seen subject \code{i}, it can use that subject’s data to adjust the \emph{population} intercept to be more specific to that subjects results. diff --git a/man/details_poisson_reg_stan_glmer.Rd b/man/details_poisson_reg_stan_glmer.Rd index ef1065ada..e34078846 100644 --- a/man/details_poisson_reg_stan_glmer.Rd +++ b/man/details_poisson_reg_stan_glmer.Rd @@ -63,7 +63,7 @@ linear predictor (\verb{\eta}) for a random intercept: \if{html}{\out{
}}\preformatted{\eta_\{i\} = (\beta_0 + b_\{0i\}) + \beta_1x_\{i1\} }\if{html}{\out{
}} -where $i$ denotes the \code{i}th independent experimental unit +where \code{i} denotes the \code{i}th independent experimental unit (e.g. subject). When the model has seen subject \code{i}, it can use that subject’s data to adjust the \emph{population} intercept to be more specific to that subjects results. diff --git a/tests/testthat/_snaps/args_and_modes.md b/tests/testthat/_snaps/args_and_modes.md index 175f926da..af1ee702b 100644 --- a/tests/testthat/_snaps/args_and_modes.md +++ b/tests/testthat/_snaps/args_and_modes.md @@ -108,16 +108,16 @@ Code linear_reg() %>% set_engine() Condition - Error in `set_engine.model_spec()`: - ! argument "engine" is missing, with no default + Error in `set_engine()`: + ! Missing engine. Possible mode/engine combinations are: quantile regression {quantreg} and regression {lm, glm, glmnet, stan, spark, keras, brulee}. --- Code proportional_hazards() %>% set_engine() Condition - Error in `set_engine.model_spec()`: - ! argument "engine" is missing, with no default + Error in `set_engine()`: + ! No known engines for `proportional_hazards()`. # set_* functions error when input isn't model_spec diff --git a/tests/testthat/_snaps/mars.md b/tests/testthat/_snaps/mars.md index 5e5d9c4ce..75ef9ba90 100644 --- a/tests/testthat/_snaps/mars.md +++ b/tests/testthat/_snaps/mars.md @@ -19,8 +19,8 @@ Code translate(mars(mode = "regression") %>% set_engine()) Condition - Error in `set_engine.model_spec()`: - ! argument "engine" is missing, with no default + Error in `set_engine()`: + ! Missing engine. Possible mode/engine combinations are: classification {earth} and regression {earth}. --- diff --git a/tests/testthat/_snaps/mlp_keras.md b/tests/testthat/_snaps/mlp_keras.md index f38dbea23..b64bfbda6 100644 --- a/tests/testthat/_snaps/mlp_keras.md +++ b/tests/testthat/_snaps/mlp_keras.md @@ -6,3 +6,13 @@ Error: ! object 'novar' not found +# all keras activation functions + + Code + mlp(mode = "classification", hidden_units = 2, penalty = 0.01, epochs = 2, + activation = "invalid") %>% set_engine("keras", verbose = 0) %>% parsnip::fit( + Class ~ A + B, data = modeldata::two_class_dat) + Condition + Error in `parsnip::keras_mlp()`: + ! `activation` should be one of: elu, exponential, gelu, hard_sigmoid, linear, relu, selu, sigmoid, softmax, softplus, softsign, swish, and tanh, not "invalid". + diff --git a/tests/testthat/_snaps/multinom_reg.md b/tests/testthat/_snaps/multinom_reg.md index 89d053fd7..78d2ef3ad 100644 --- a/tests/testthat/_snaps/multinom_reg.md +++ b/tests/testthat/_snaps/multinom_reg.md @@ -37,8 +37,8 @@ Code multinom_reg(penalty = 0.1) %>% set_engine() Condition - Error in `set_engine.model_spec()`: - ! argument "engine" is missing, with no default + Error in `set_engine()`: + ! Missing engine. Possible mode/engine combinations are: classification {glmnet, spark, keras, nnet, brulee}. # check_args() works diff --git a/tests/testthat/_snaps/nullmodel.md b/tests/testthat/_snaps/nullmodel.md index 3465197b2..79ac405b1 100644 --- a/tests/testthat/_snaps/nullmodel.md +++ b/tests/testthat/_snaps/nullmodel.md @@ -3,8 +3,8 @@ Code translate(null_model(mode = "regression") %>% set_engine()) Condition - Error in `set_engine.model_spec()`: - ! argument "engine" is missing, with no default + Error in `set_engine()`: + ! Missing engine. Possible mode/engine combinations are: classification {parsnip} and regression {parsnip}. --- From 6d60c507f0deee9510b3b2578ce573cbe5291885 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E2=80=98topepo=E2=80=99?= Date: Tue, 11 Feb 2025 15:39:39 -0500 Subject: [PATCH 5/5] skip for upcoming dials release --- man/parsnip-package.Rd | 2 +- tests/testthat/test-mlp_keras.R | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/man/parsnip-package.Rd b/man/parsnip-package.Rd index 84b9a622d..2e074dc3b 100644 --- a/man/parsnip-package.Rd +++ b/man/parsnip-package.Rd @@ -30,7 +30,7 @@ Authors: Other contributors: \itemize{ \item Emil Hvitfeldt \email{emil.hvitfeldt@posit.co} [contributor] - \item Posit Software, PBC [copyright holder, funder] + \item Posit Software, PBC (03wc8by49) [copyright holder, funder] } } diff --git a/tests/testthat/test-mlp_keras.R b/tests/testthat/test-mlp_keras.R index 8581a2530..2b7ba2cc3 100644 --- a/tests/testthat/test-mlp_keras.R +++ b/tests/testthat/test-mlp_keras.R @@ -252,6 +252,7 @@ test_that('all keras activation functions', { skip_on_cran() skip_if_not_installed("keras") skip_if_not_installed("modeldata") + skip_if_not_installed("dials", minimum_version = "1.3.0.9000") skip_if(!is_tf_ok()) act <- parsnip:::keras_activations()