-
Notifications
You must be signed in to change notification settings - Fork 92
prompt on unavailable extension packages #732
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 14 commits
4b04aa6
3bd57a8
e703371
d33c2eb
17b8203
31db1aa
6174fa1
f3ff5f1
8c4ea8e
7070827
8843bcf
7eb956c
1ca9e60
ff84494
50180e7
8978637
f9c7e77
51ef493
4dd0fb9
6c06315
9918043
51ffc03
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -79,6 +79,72 @@ model_printer <- function(x, ...) { | |
is_missing_arg <- function(x) | ||
identical(x, quote(missing_arg())) | ||
|
||
# given a model object, return TRUE if: | ||
# * the model is supported without extensions | ||
# * the model needs an extension and it is loaded | ||
# | ||
# return FALSE if: | ||
# * the model needs an extension and it is _not_ loaded | ||
has_loaded_implementation <- function(spec_, engine_, mode_) { | ||
if (isFALSE(mode_ %in% c("regression", "censored regression", "classification"))) { | ||
mode_ <- c("regression", "censored regression", "classification") | ||
} | ||
eng_cond <- if (is.null(engine_)) {TRUE} else {quote(engine == engine_)} | ||
|
||
avail <- | ||
get_from_env(spec_) %>% | ||
dplyr::filter(mode %in% mode_, !!eng_cond) | ||
pars <- | ||
utils::read.delim(system.file("models.tsv", package = "parsnip")) %>% | ||
dplyr::filter(model == spec_, !!eng_cond, mode %in% mode_, is.na(pkg)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this an expensive check to do every time There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Certainly more expensive than our other checks. But—
library(parsnip)
# running the check
system.time(mod <- C5_rules())
#> parsnip could not locate an implementation for `C5_rules` classification model specifications using the `C5.0` engine.
#> ℹ The parsnip extension package rules implements support for this specification.
#> ℹ Please install (if needed) and load to continue.
#> user system elapsed
#> 0.038 0.001 0.039
# doesn't run the check
system.time(mod <- mod %>% set_engine("C5.0"))
#> user system elapsed
#> 0.008 0.000 0.009
# model fit to a lil fella
library(rules)
mtcars$cyl <- as.factor(mtcars$cyl)
system.time(mod %>% fit(cyl ~ ., mtcars))
#> user system elapsed
#> 0.445 0.020 0.467 Created on 2022-05-25 by the reprex package (v2.0.1) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The fact that it could be common to read this file multiple times seems like something to check and get other feedback on. We could read this file one time at There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think that's a good observation. After 9918043, parsnip now assigns the library(parsnip)
# now just assigns models.tsv to internal object:
head(parsnip:::model_info_table)
#> model mode engine pkg
#> 1 bag_mars classification earth baguette
#> 2 bag_mars regression earth baguette
#> 3 bag_tree censored regression rpart censored
#> 4 bag_tree classification C5.0 baguette
#> 5 bag_tree classification rpart baguette
#> 6 bag_tree regression rpart baguette
# running the check. used to be:
#> user system elapsed
#> 0.038 0.001 0.039
system.time(mod <- C5_rules())
#> parsnip could not locate an implementation for `C5_rules` classification model specifications using the `C5.0` engine.
#> ℹ The parsnip extension package rules implements support for this specification.
#> ℹ Please install (if needed) and load to continue.
#> user system elapsed
#> 0.033 0.001 0.033
# doesn't run the check. used to be:
#> user system elapsed
#> 0.008 0.000 0.009
system.time(mod <- mod %>% set_engine("C5.0"))
#> user system elapsed
#> 0.007 0.000 0.007
# model fit to a lil fella, for comparison
library(rules)
mtcars$cyl <- as.factor(mtcars$cyl)
system.time(mod %>% fit(cyl ~ ., mtcars))
#> user system elapsed
#> 0.440 0.025 0.464 Created on 2022-05-27 by the reprex package (v2.0.1) I opted not to ditch that |
||
|
||
if (nrow(pars) > 0 || nrow(avail) > 0) { | ||
return(TRUE) | ||
} | ||
|
||
FALSE | ||
} | ||
|
||
# construct a message informing the user that there are no | ||
# implementations for the current model spec / mode / engine. | ||
# | ||
# if there's a "pre-registered" extension supporting that setup, | ||
# nudge the user to install/load it. | ||
prompt_missing_implementation <- function(spec_, engine_, mode_) { | ||
simonpcouch marked this conversation as resolved.
Show resolved
Hide resolved
|
||
avail <- | ||
show_engines(spec_) %>% | ||
dplyr::filter(mode == mode_, engine == engine_) | ||
all <- | ||
utils::read.delim(system.file("models.tsv", package = "parsnip")) %>% | ||
dplyr::filter(model == spec_, mode == mode_, engine == engine_, !is.na(pkg)) %>% | ||
dplyr::select(-model) | ||
|
||
if (identical(mode_, "unknown")) { | ||
mode_ <- "" | ||
} | ||
|
||
msg <- | ||
glue::glue( | ||
"parsnip could not locate an implementation for `{spec_}` {mode_} model ", | ||
"specifications using the `{engine_}` engine." | ||
simonpcouch marked this conversation as resolved.
Show resolved
Hide resolved
|
||
) | ||
|
||
if (nrow(avail) == 0 && nrow(all) > 0) { | ||
juliasilge marked this conversation as resolved.
Show resolved
Hide resolved
|
||
msg <- | ||
glue::glue_collapse(c( | ||
msg, | ||
glue::glue_collapse(c( | ||
"\nThe parsnip extension package {", | ||
cli::col_yellow(all$pkg[[1]]), | ||
"} implements support for this specification. \n", | ||
"Please install (if needed) and load to continue.\n" | ||
)) | ||
simonpcouch marked this conversation as resolved.
Show resolved
Hide resolved
|
||
)) | ||
} | ||
|
||
msg | ||
} | ||
|
||
|
||
#' Print the model call | ||
#' | ||
|
@@ -89,18 +155,10 @@ is_missing_arg <- function(x) | |
show_call <- function(object) { | ||
object$method$fit$args <- | ||
map(object$method$fit$args, convert_arg) | ||
if ( | ||
simonpcouch marked this conversation as resolved.
Show resolved
Hide resolved
|
||
is.null(object$method$fit$func["pkg"]) || | ||
is.na(object$method$fit$func["pkg"]) | ||
) { | ||
res <- call2(object$method$fit$func["fun"], !!!object$method$fit$args) | ||
} else { | ||
res <- | ||
call2(object$method$fit$func["fun"], | ||
!!!object$method$fit$args, | ||
.ns = object$method$fit$func["pkg"]) | ||
} | ||
res | ||
|
||
call2(object$method$fit$func["fun"], | ||
!!!object$method$fit$args, | ||
.ns = object$method$fit$func["pkg"]) | ||
} | ||
|
||
convert_arg <- function(x) { | ||
|
@@ -110,7 +168,6 @@ convert_arg <- function(x) { | |
x | ||
} | ||
|
||
|
||
levels_from_formula <- function(f, dat) { | ||
if (inherits(dat, "tbl_spark")) | ||
res <- NULL | ||
|
@@ -181,10 +238,14 @@ update_dot_check <- function(...) { | |
#' @export | ||
#' @keywords internal | ||
#' @rdname add_on_exports | ||
new_model_spec <- function(cls, args, eng_args, mode, method, engine) { | ||
new_model_spec <- function(cls, args, eng_args, mode, method, engine, new = TRUE) { | ||
simonpcouch marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
check_spec_mode_engine_val(cls, engine, mode) | ||
|
||
if ((!has_loaded_implementation(cls, engine, mode)) && new) { | ||
rlang::inform(prompt_missing_implementation(cls, engine, mode)) | ||
} | ||
|
||
out <- list(args = args, eng_args = eng_args, | ||
mode = mode, method = method, engine = engine) | ||
class(out) <- make_classes(cls) | ||
|
Uh oh!
There was an error while loading. Please reload this page.