Skip to content

Commit c54f07b

Browse files
authored
improve prompting about missing implementations (#793)
* move missing extension prompt from `new_model_spec` to `print_model_spec` also, deprecates `check_missing_spec` argument * tighten logic for no-engines-in-parsnip case this code used to miss the edge case where a non-relevant extension package was loaded. e.g.: ``` bag_tree() %>% set_engine("C5.0") #> parsnip could not locate an implementation for `bag_tree` model specifications #> using the `C5.0` engine. #> Bagged Decision Tree Model Specification (unknown) #> #> Main Arguments: #> cost_complexity = 0 #> min_n = 2 #> #> Computational engine: C5.0 library(censored) bag_tree() %>% set_engine("C5.0") #> Error in `check_spec_mode_engine_val()`: #> ! Engine 'C5.0' is not supported for `bag_tree()`. See `show_engines('bag_tree')`. ``` * tighten logic with modes, transition prompts from `rlang` -> `cli` * prompt on missing implementation in `fit` also: * fixes duplicated packages in prompt * appends a newline regardless of whether a specific extension is recommended * allows passing additional arguments to `prompt` * update snapshots + tests mostly undoing changes made for 732. * add tests still need to test interactions with extensions--this will have to live in extratests * re`document` * use `cli` formatting for `.pkg`s * integrate `default` attributes for engine and mode * migrate arg attributes to new model spec slots e.g. `attr(model_spec$mode, "default")` now lives at `model_spec$user_specified_mode`. a likely _less_ breaking change and lives more visibly in the object structure. * test fixes + updates, swimming in edge case spaghetti * re`document()` with new `new_model_spec()` args * address some review comments * transition `model_info_table` read to a helper * `implementation_exists_somewhere` -> `spec_is_possible` * comment on `*_filter_condition` helpers * minimal testing for old/external objects * address remainder of review comments * comment on checking strategy * name arguments to `prompt_missing_implementation` * mark parsnip with `.pkg` cli tag * export and document model spec checking functions also, renames `has_loaded_implementation` -> `spec_is_loaded`
1 parent fdde60a commit c54f07b

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

47 files changed

+523
-122
lines changed

NAMESPACE

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -246,6 +246,7 @@ export(predict_time)
246246
export(predict_time.model_fit)
247247
export(prepare_data)
248248
export(print_model_spec)
249+
export(prompt_missing_implementation)
249250
export(proportional_hazards)
250251
export(rand_forest)
251252
export(repair_call)
@@ -271,6 +272,8 @@ export(show_call)
271272
export(show_engines)
272273
export(show_fit)
273274
export(show_model_info)
275+
export(spec_is_loaded)
276+
export(spec_is_possible)
274277
export(stan_conf_int)
275278
export(surv_reg)
276279
export(survival_reg)

R/aaa_models.R

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,15 @@ pred_types <-
3737

3838
# ------------------------------------------------------------------------------
3939

40+
read_model_info_table <- function() {
41+
model_info_table <-
42+
utils::read.delim(system.file("models.tsv", package = "parsnip"))
43+
44+
model_info_table
45+
}
46+
47+
# ------------------------------------------------------------------------------
48+
4049
#' Working with the parsnip model environment
4150
#'
4251
#' These functions read and write to the environment where the package stores
@@ -220,7 +229,14 @@ check_spec_mode_engine_val <- function(cls, eng, mode) {
220229

221230
# Cases where the model definition is in parsnip but all of the engines
222231
# are contained in a different package
223-
if (nrow(model_info) == 0) {
232+
model_info_parsnip_only <-
233+
dplyr::inner_join(
234+
read_model_info_table() %>% dplyr::filter(is.na(pkg)) %>% dplyr::select(-pkg),
235+
model_info %>% dplyr::mutate(model = cls),
236+
by = c("model", "engine", "mode")
237+
)
238+
239+
if (nrow(model_info_parsnip_only) == 0) {
224240
check_mode_with_no_engine(cls, mode)
225241
return(invisible(NULL))
226242
}

R/arguments.R

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -71,9 +71,10 @@ set_args.model_spec <- function(object, ...) {
7171
args = object$args,
7272
eng_args = object$eng_args,
7373
mode = object$mode,
74+
user_specified_mode = object$user_specified_mode,
7475
method = NULL,
7576
engine = object$engine,
76-
check_missing_spec = FALSE
77+
user_specified_engine = object$user_specified_engine
7778
)
7879
}
7980

@@ -97,8 +98,18 @@ set_mode.model_spec <- function(object, mode) {
9798
spec_modes <- rlang::env_get(get_model_env(), paste0(cls, "_modes"))
9899
stop_incompatible_mode(spec_modes, cls = cls)
99100
}
100-
check_spec_mode_engine_val(cls, object$engine, mode)
101+
102+
# determine if the model specification could feasibly match any entry
103+
# in the union of the parsnip model environment and model_info_table.
104+
# if not, trigger an error based on the (possibly inferred) model spec slots.
105+
if (!spec_is_possible(cls,
106+
object$engine, object$user_specified_engine,
107+
mode, user_specified_mode = TRUE)) {
108+
check_spec_mode_engine_val(cls, object$engine, mode)
109+
}
110+
101111
object$mode <- mode
112+
object$user_specified_mode <- TRUE
102113
object
103114
}
104115

R/bag_mars.R

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,8 +37,10 @@ bag_mars <-
3737
args = args,
3838
eng_args = NULL,
3939
mode = mode,
40+
user_specified_mode = !missing(mode),
4041
method = NULL,
41-
engine = engine
42+
engine = engine,
43+
user_specified_engine = !missing(engine)
4244
)
4345
}
4446

R/bag_tree.R

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,8 +41,10 @@ bag_tree <-
4141
args = args,
4242
eng_args = NULL,
4343
mode = mode,
44+
user_specified_mode = !missing(mode),
4445
method = NULL,
45-
engine = engine
46+
engine = engine,
47+
user_specified_engine = !missing(engine)
4648
)
4749
}
4850

R/bart.R

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,8 +85,10 @@ bart <-
8585
args = args,
8686
eng_args = NULL,
8787
mode = mode,
88+
user_specified_mode = !missing(mode),
8889
method = NULL,
89-
engine = engine
90+
engine = engine,
91+
user_specified_engine = !missing(engine)
9092
)
9193
}
9294

R/boost_tree.R

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -74,9 +74,11 @@ boost_tree <-
7474
"boost_tree",
7575
args,
7676
eng_args = NULL,
77-
mode,
77+
mode = mode,
78+
user_specified_mode = !missing(mode),
7879
method = NULL,
79-
engine = engine
80+
engine = engine,
81+
user_specified_engine = !missing(engine)
8082
)
8183
}
8284

R/c5_rules.R

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,8 +58,10 @@ C5_rules <-
5858
args = args,
5959
eng_args = NULL,
6060
mode = mode,
61+
user_specified_mode = !missing(mode),
6162
method = NULL,
62-
engine = engine
63+
engine = engine,
64+
user_specified_engine = !missing(engine)
6365
)
6466
}
6567

R/cubist_rules.R

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,8 +84,10 @@ cubist_rules <-
8484
args = args,
8585
eng_args = NULL,
8686
mode = mode,
87+
user_specified_mode = !missing(mode),
8788
method = NULL,
88-
engine = engine
89+
engine = engine,
90+
user_specified_engine = !missing(engine)
8991
)
9092
}
9193

R/decision_tree.R

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,8 +46,10 @@ decision_tree <-
4646
args = args,
4747
eng_args = NULL,
4848
mode = mode,
49+
user_specified_mode = !missing(mode),
4950
method = NULL,
50-
engine = engine
51+
engine = engine,
52+
user_specified_engine = !missing(engine)
5153
)
5254
}
5355

0 commit comments

Comments
 (0)