set_model_arg() errors uninformatively when func isn't a list

[chillerb]

The problem

I have a weird problem, where I registered a new parsnip model, but on calling tune_grid it results in an error message.
The issue might be my own fault, but the error message appears a little bit cryptic to me and I can't pinpoint the actual problem.
The error didn't appear for the MDA discrim_mixture tutorial though.

mymodel_spec <- mymodel_reg(mixture = 0.5, penalty = tune(), penalty_z = 0.5) %>%
  set_engine(eng = "glmnet", z = z)

# tune parameters on val_cv
wf <- workflow() %>%
  add_model(mymodel_spec) %>%
  add_formula(response ~ . - group)

res <- tune_grid(wf, val_cv)

best <- select_best(res)

Error in `mutate()` at magrittr/R/pipe.R:136:3:
ℹ In argument: `object = purrr::map(call_info, eval_call_info)`.
Caused by error in `purrr::map()`:
ℹ In index: 1.
Caused by error in `x$fun`:
! $ operator is invalid for atomic vectors
Run `rlang::last_trace()` to see where the error occurred.

 Error in mutate(., object = purrr::map(call_info, eval_call_info)) : 
Caused by error in `purrr::map()`:
ℹ In index: 1.
Caused by error in `x$fun`:
! $ operator is invalid for atomic vectors
 
34.
	
stop(fallback)
 
33.
	
signal_abort(cnd, .file)
 
32.
	
abort(message, class = error_class, parent = parent, call = error_call) at
 conditions.R#235
31.
	
(function (cnd) 
{
    local_error_context(dots, i = frame[[i_sym]], mask = mask)
    if (inherits(cnd, "dplyr:::internal_error")) { ...
 
30.
	
signalCondition(cnd)
 
29.
	
signal_abort(cnd, .file)
 
28.
	
rlang::abort(message, ..., call = call, use_cli_format = TRUE, 
    .frame = .frame) at
 rlang.R#45
27.
	
cli::cli_abort(message, location = i, name = name, parent = cnd, 
    call = error_call, class = "purrr_error_indexed") at
 map.R#215
26.
	
h(simpleError(msg, call))
 
25.
	
.handleSimpleError(function (cnd) 
{
    if (i == 0L) {
    } ... at
 extract.R#117
24.
	
paste0("Error when calling ", x$fun, "(): ", as.character(res)) at
 extract.R#117
23.
	
stop(paste0("Error when calling ", x$fun, "(): ", as.character(res))) at
 extract.R#117
22.
	
.f(.x[[i]], ...)
 
21.
	
call_with_cleanup(map_impl, environment(), .type, .progress, 
    n, names, i) at
 map.R#174
20.
	
withCallingHandlers(expr, error = function(cnd) {
    if (i == 0L) {
    }
    else { ... at
 map.R#201
19.
	
with_indexed_errors(i = i, names = names, error_call = .purrr_error_call, 
    call_with_cleanup(map_impl, environment(), .type, .progress, 
        n, names, i)) at
 map.R#174
18.
	
map_("list", .x, .f, ..., .progress = .progress) at
 map.R#129
17.
	
purrr::map(call_info, eval_call_info)
 
16.
	
eval() at
 data-mask.R#94
15.
	
mask$eval_all_mutate(quo) at
 mutate.R#380
14.
	
mutate_col(dots[[i]], data, mask, new_columns) at
 mutate.R#273
13.
	
withCallingHandlers(for (i in seq_along(dots)) {
    poke_error_context(dots, i, mask = mask)
    context_poke("column", old_current_column)
    new_columns <- mutate_col(dots[[i]], data, mask, new_columns) ... at
 mutate.R#268
12.
	
mutate_cols(.data, dplyr_quosures(...), by) at
 mutate.R#181
11.
	
mutate.data.frame(., object = purrr::map(call_info, eval_call_info)) at
 mutate.R#146
10.
	
mutate(., object = purrr::map(call_info, eval_call_info)) at
 pipe.R#136
9.
	
dplyr::inner_join(tuning_param %>% dplyr::select(-tunable, -component_id), 
    all_args, by = c("name", "source", "component")) %>% mutate(object = purrr::map(call_info, 
    eval_call_info)) at
 extract.R#88
8.
	
extract_parameter_set_dials.model_spec(model) at
 extract.R#105
7.
	
extract_parameter_set_dials(model) at
 extract.R#191
6.
	
extract_parameter_set_dials.workflow(wflow) at
 extract.R#105
5.
	
hardhat::extract_parameter_set_dials(wflow) at
 checks.R#146
4.
	
check_parameters(workflow, pset = pset, data = resamples$splits[[1]]$data, 
    grid_names = names(grid)) at
 tune_grid.R#335
3.
	
tune_grid_workflow(object, resamples = resamples, grid = grid, 
    metrics = metrics, eval_time = eval_time, pset = param_info, 
    control = control) at
 tune_grid.R#305
2.
	
tune_grid.workflow(wf, val_cv) at
 tune_grid.R#243
1.
	
tune_grid(wf, val_cv)

Reproducible example

Sorry, maybe I will add a fully reproducible example later.

[chillerb]

Might have also been related to #1252

[EmilHvitfeldt]

Hello @chillerb 👋

We would sadly need a more complete reprex to be able to help you

[chillerb]

Hi @EmilHvitfeldt,
sorry, until now, I was not able to reproduce the issue again, and everything is working as expected now.
I was pretty confused what was going on, and I'm still not sure what caused the issue.
I hoped that maybe the stacktrace could be of any help at least.
If I run into it again, I will try to look deeper into it.

[chillerb]

Hi again @EmilHvitfeldt ,
I actually encountered the error again today, and I finally figured out what was causing it!

You see, I registered a model argument like this:

  set_model_arg(
    model = "my_reg",
    eng = "my_engine",
    parsnip = "mixture",
    original = "alpha",
    func = c(pkg = "dials", fun = "mixture"),
    has_submodel = FALSE
  )

When I should have registered it like this:

  set_model_arg(
    model = "my_reg",
    eng = "my_engine",
    parsnip = "mixture",
    original = "alpha",
    func = list(pkg = "dials", fun = "mixture"),
    has_submodel = FALSE
  )

It took me forever to notice the difference: The error occurs when func is passed as character vector instead of a list, therefore not allowing access via the $ operator. In hindsight, it was all there in the error message...

However, I still think this should be addressed by additional type checks and a clearer error message or automatic type conversions. Especially, since the documentation for set_model_arg states:

func: A named character vector that describes how to call a function. func should have elements pkg and fun. The former is optional but is recommended and the latter is required. For example, c(pkg = "stats", fun = "lm") would be used to invoke the usual linear regression function. In some cases, it is helpful to use c(fun = "predict") when using a package's predict method.

[EmilHvitfeldt]

Thank you! Good find!

That should be an easy fix on our side