From d244558823ccf9587c3e122d0fe6a86df223c060 Mon Sep 17 00:00:00 2001 From: simonpcouch Date: Thu, 8 Sep 2022 11:30:07 -0400 Subject: [PATCH 1/3] prompt on `add_model()` when missing implementation --- DESCRIPTION | 4 +++- R/fit-action-model.R | 17 ++++++++++++++ tests/testthat/_snaps/fit-action-model.md | 28 +++++++++++++++++++++++ tests/testthat/test-fit-action-model.R | 12 ++++++++++ 4 files changed, 60 insertions(+), 1 deletion(-) diff --git a/DESCRIPTION b/DESCRIPTION index d4440ebf..286cefcf 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -23,7 +23,7 @@ Imports: glue (>= 1.6.2), hardhat (>= 1.2.0), lifecycle (>= 1.0.1), - parsnip (>= 1.0.0), + parsnip (>= 1.0.1.9000), rlang (>= 1.0.3), tidyselect (>= 1.1.2), vctrs (>= 0.4.1) @@ -45,6 +45,8 @@ Config/Needs/website: tidyr, tidyverse/tidytemplate, yardstick +Remotes: + tidymodels/parsnip Config/testthat/edition: 3 Encoding: UTF-8 Roxygen: list(markdown = TRUE) diff --git a/R/fit-action-model.R b/R/fit-action-model.R index 4465afbc..8ffd0cda 100644 --- a/R/fit-action-model.R +++ b/R/fit-action-model.R @@ -190,5 +190,22 @@ new_action_model <- function(spec, formula, ..., call = caller_env()) { abort("`formula` must be a formula, or `NULL`.", call = call) } + if (!parsnip::spec_is_loaded( + cls = class(spec)[1], + engine = spec$engine, + user_specified_engine = spec$user_specified_engine, + mode = spec$mode, + user_specified_mode = spec$user_specified_mode + )) { + parsnip::prompt_missing_implementation( + cls = class(spec)[1], + engine = spec$engine, + user_specified_engine = spec$user_specified_engine, + mode = spec$mode, + user_specified_mode = spec$user_specified_mode, + prompt = cli::cli_inform + ) + } + new_action_fit(spec = spec, formula = formula, subclass = "action_model") } diff --git a/tests/testthat/_snaps/fit-action-model.md b/tests/testthat/_snaps/fit-action-model.md index 338d8d91..d3b0533f 100644 --- a/tests/testthat/_snaps/fit-action-model.md +++ b/tests/testthat/_snaps/fit-action-model.md @@ -15,6 +15,34 @@ ! `spec` must have a known mode. i Set the mode of `spec` by using `parsnip::set_mode()` or by setting the mode directly in the parsnip specification function. +# prompt on spec without a loaded implementation (#174) + + Code + add_model(workflow, mod) + Message + ! parsnip could not locate an implementation for `bag_tree` regression model specifications using the `rpart` engine. + i The parsnip extension package baguette implements support for this specification. + i Please install (if needed) and load to continue. + Output + == Workflow ==================================================================== + Preprocessor: None + Model: bag_tree() + + -- Model ----------------------------------------------------------------------- + Message + ! parsnip could not locate an implementation for `bag_tree` regression model specifications using the `rpart` engine. + i The parsnip extension package baguette implements support for this specification. + i Please install (if needed) and load to continue. + Output + Bagged Decision Tree Model Specification (regression) + + Main Arguments: + cost_complexity = 0 + min_n = 2 + + Computational engine: rpart + + # cannot add two models Code diff --git a/tests/testthat/test-fit-action-model.R b/tests/testthat/test-fit-action-model.R index 4590c72c..8b2c52ec 100644 --- a/tests/testthat/test-fit-action-model.R +++ b/tests/testthat/test-fit-action-model.R @@ -22,6 +22,18 @@ test_that("model must contain a known mode (#160)", { }) }) +test_that("prompt on spec without a loaded implementation (#174)", { + mod <- parsnip::bag_tree() %>% + parsnip::set_engine("rpart") %>% + parsnip::set_mode("regression") + + workflow <- workflow() + + expect_snapshot({ + add_model(workflow, mod) + }) +}) + test_that("cannot add two models", { mod <- parsnip::linear_reg() mod <- parsnip::set_engine(mod, "lm") From 5f7db8c66540558b5a6d1ba6c361cecd7cb43c31 Mon Sep 17 00:00:00 2001 From: simonpcouch Date: Tue, 13 Sep 2022 09:15:35 -0400 Subject: [PATCH 2/3] transition first argument --- R/fit-action-model.R | 17 ++--------------- 1 file changed, 2 insertions(+), 15 deletions(-) diff --git a/R/fit-action-model.R b/R/fit-action-model.R index 8ffd0cda..a70149d4 100644 --- a/R/fit-action-model.R +++ b/R/fit-action-model.R @@ -190,21 +190,8 @@ new_action_model <- function(spec, formula, ..., call = caller_env()) { abort("`formula` must be a formula, or `NULL`.", call = call) } - if (!parsnip::spec_is_loaded( - cls = class(spec)[1], - engine = spec$engine, - user_specified_engine = spec$user_specified_engine, - mode = spec$mode, - user_specified_mode = spec$user_specified_mode - )) { - parsnip::prompt_missing_implementation( - cls = class(spec)[1], - engine = spec$engine, - user_specified_engine = spec$user_specified_engine, - mode = spec$mode, - user_specified_mode = spec$user_specified_mode, - prompt = cli::cli_inform - ) + if (!parsnip::spec_is_loaded(spec = spec)) { + parsnip::prompt_missing_implementation(spec = spec, prompt = cli::cli_inform) } new_action_fit(spec = spec, formula = formula, subclass = "action_model") From 827403e0b9adc0fcc4dc29e4782f3b383a6e6d0b Mon Sep 17 00:00:00 2001 From: simonpcouch Date: Thu, 15 Sep 2022 09:28:09 -0400 Subject: [PATCH 3/3] transition message to error I'd prefer the referenced call in the second snapshot to be `workflow` rather than `add_model`, though this would be a good bit more invasive. --- R/fit-action-model.R | 6 ++++- tests/testthat/_snaps/fit-action-model.md | 30 +++++++++-------------- tests/testthat/test-fit-action-model.R | 6 ++--- 3 files changed, 18 insertions(+), 24 deletions(-) diff --git a/R/fit-action-model.R b/R/fit-action-model.R index a70149d4..d87016e6 100644 --- a/R/fit-action-model.R +++ b/R/fit-action-model.R @@ -191,7 +191,11 @@ new_action_model <- function(spec, formula, ..., call = caller_env()) { } if (!parsnip::spec_is_loaded(spec = spec)) { - parsnip::prompt_missing_implementation(spec = spec, prompt = cli::cli_inform) + parsnip::prompt_missing_implementation( + spec = spec, + prompt = cli::cli_abort, + call = call + ) } new_action_fit(spec = spec, formula = formula, subclass = "action_model") diff --git a/tests/testthat/_snaps/fit-action-model.md b/tests/testthat/_snaps/fit-action-model.md index d3b0533f..ad5d41cb 100644 --- a/tests/testthat/_snaps/fit-action-model.md +++ b/tests/testthat/_snaps/fit-action-model.md @@ -19,29 +19,21 @@ Code add_model(workflow, mod) - Message - ! parsnip could not locate an implementation for `bag_tree` regression model specifications using the `rpart` engine. + Condition + Error in `add_model()`: + ! parsnip could not locate an implementation for `bag_tree` regression model specifications. i The parsnip extension package baguette implements support for this specification. i Please install (if needed) and load to continue. - Output - == Workflow ==================================================================== - Preprocessor: None - Model: bag_tree() - - -- Model ----------------------------------------------------------------------- - Message - ! parsnip could not locate an implementation for `bag_tree` regression model specifications using the `rpart` engine. + +--- + + Code + workflow(spec = mod) + Condition + Error in `add_model()`: + ! parsnip could not locate an implementation for `bag_tree` regression model specifications. i The parsnip extension package baguette implements support for this specification. i Please install (if needed) and load to continue. - Output - Bagged Decision Tree Model Specification (regression) - - Main Arguments: - cost_complexity = 0 - min_n = 2 - - Computational engine: rpart - # cannot add two models diff --git a/tests/testthat/test-fit-action-model.R b/tests/testthat/test-fit-action-model.R index 8b2c52ec..c87543ac 100644 --- a/tests/testthat/test-fit-action-model.R +++ b/tests/testthat/test-fit-action-model.R @@ -24,14 +24,12 @@ test_that("model must contain a known mode (#160)", { test_that("prompt on spec without a loaded implementation (#174)", { mod <- parsnip::bag_tree() %>% - parsnip::set_engine("rpart") %>% parsnip::set_mode("regression") workflow <- workflow() - expect_snapshot({ - add_model(workflow, mod) - }) + expect_snapshot(error = TRUE, add_model(workflow, mod)) + expect_snapshot(error = TRUE, workflow(spec = mod)) }) test_that("cannot add two models", {