Skip to content
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

metric_tweak and metric_set doesn't work together #351

Closed
EmilHvitfeldt opened this issue Jan 9, 2023 · 3 comments · Fixed by #365
Closed

metric_tweak and metric_set doesn't work together #351

EmilHvitfeldt opened this issue Jan 9, 2023 · 3 comments · Fixed by #365
Labels
bug an unexpected problem or unintended behavior

Comments

@EmilHvitfeldt
Copy link
Member

EmilHvitfeldt commented Jan 9, 2023

Originally found here https://stackoverflow.com/questions/75034496/tidymodels-f-meas-metric-tweak-error-on-metric-set

library(yardstick)
#> For binary classification, the first factor level is assumed to be the event.
#> Use the argument `event_level = "second"` to alter this as needed.

f_meas_weighted <- metric_tweak("f_meas_weighted", f_meas, estimator = "macro_weighted")

f_meas_weighted(iris, truth = Species, estimate = Species)
#> # A tibble: 1 × 3
#>   .metric         .estimator     .estimate
#>   <chr>           <chr>              <dbl>
#> 1 f_meas_weighted macro_weighted         1

metric_set(f_meas_weighted)(iris, truth = Species, estimate = Species)
#> Error in `metric_set()`:
#> ! Failed to compute `f_meas_weighted()`.
#> Caused by error in `f_meas.data.frame()`:
#> ! formal argument "estimator" matched by multiple actual arguments

#> Backtrace:
#>     ▆
#>  1. └─metric_set(f_meas_weighted)(iris, truth = Species, estimate = Species)
#>  2.   └─base::mapply(...) at yardstick/R/metrics.R:351:6
#>  3.     └─yardstick (local) `<fn>`(dots[[1L]][[1L]], dots[[2L]][[1L]])
#>  4.       └─base::tryCatch(...) at yardstick/R/metrics.R:561:2
#>  5.         └─base (local) tryCatchList(expr, classes, parentenv, handlers)
#>  6.           └─base (local) tryCatchOne(expr, names, parentenv, handlers[[1L]])
#>  7.             └─value[[3L]](cond)
#>  8.               └─rlang::abort(message, parent = cnd, call = call("metric_set")) at yardstick/R/metrics.R:567:6

Created on 2023-01-09 with reprex v2.0.2

@EmilHvitfeldt EmilHvitfeldt added the bug an unexpected problem or unintended behavior label Jan 9, 2023
@DavisVaughan
Copy link
Member

Is this really a bug? The metric_set() result has an argument for estimator that gets passed through. You are supposed to do

library(yardstick)
#> For binary classification, the first factor level is assumed to be the event.
#> Use the argument `event_level = "second"` to alter this as needed.

f_meas_weighted <- metric_tweak("f_meas_weighted", f_meas)

set <- metric_set(f_meas_weighted)

set(iris, truth = Species, estimate = Species, estimator = "macro_weighted")
#> # A tibble: 1 × 3
#>   .metric         .estimator     .estimate
#>   <chr>           <chr>              <dbl>
#> 1 f_meas_weighted macro_weighted         1

Created on 2023-02-08 with reprex v2.0.2.9000

@EmilHvitfeldt
Copy link
Member Author

EmilHvitfeldt commented Feb 8, 2023

But that assumes that you want to set the same estimator for all the metrics in the set, which I don't know is something we should force if we can avoid it

Main

library(yardstick)
#> For binary classification, the first factor level is assumed to be the event.
#> Use the argument `event_level = "second"` to alter this as needed.

j_index_macro <- metric_tweak("j_index_macro", j_index, estimator = "macro")
j_index_micro <- metric_tweak("j_index_micro", j_index, estimator = "micro")

set <- metric_set(accuracy, j_index_macro, j_index_micro)

set(iris, truth = Species, estimate = Species)
#> Error in `metric_set()`:
#> ! Failed to compute `j_index_macro()`.
#> Caused by error in `j_index.data.frame()`:
#> ! formal argument "estimator" matched by multiple actual arguments

#> Backtrace:
#>     ▆
#>  1. └─yardstick (local) set(iris, truth = Species, estimate = Species)
#>  2.   └─base::mapply(...) at yardstick/R/metrics.R:351:6
#>  3.     └─yardstick (local) `<fn>`(dots[[1L]][[2L]], dots[[2L]][[2L]])
#>  4.       └─base::tryCatch(...) at yardstick/R/metrics.R:561:2
#>  5.         └─base (local) tryCatchList(expr, classes, parentenv, handlers)
#>  6.           └─base (local) tryCatchOne(expr, names, parentenv, handlers[[1L]])
#>  7.             └─value[[3L]](cond)
#>  8.               └─rlang::abort(message, parent = cnd, call = call("metric_set")) at yardstick/R/metrics.R:567:6

PR

library(yardstick)
#> For binary classification, the first factor level is assumed to be the event.
#> Use the argument `event_level = "second"` to alter this as needed.

j_index_macro <- metric_tweak("j_index_macro", j_index, estimator = "macro")
j_index_micro <- metric_tweak("j_index_micro", j_index, estimator = "micro")

set <- metric_set(accuracy, j_index_macro, j_index_micro)

set(iris, truth = Species, estimate = Species)
#> # A tibble: 3 × 3
#>   .metric       .estimator .estimate
#>   <chr>         <chr>          <dbl>
#> 1 accuracy      multiclass         1
#> 2 j_index_macro macro              1
#> 3 j_index_micro micro              1

@github-actions
Copy link

github-actions bot commented May 4, 2023

This issue has been automatically locked. If you believe you have found a related problem, please file a new issue (with a reprex: https://reprex.tidyverse.org) and link to this issue.

@github-actions github-actions bot locked and limited conversation to collaborators May 4, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug an unexpected problem or unintended behavior
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants