Check type of `na_rm`? - Githubissues

mikemahoney218 commented 1 year ago

Feature

Right now, passing anything that's truthy to na_rm will remove NA values, while values that can't be casted to logical crash in an if statement:

yardstick::rmse_vec(c(1, 3, NA), c(2, 5, 2), na_rm = 3)
#> [1] 1.581139
yardstick::rmse_vec(c(1, 3, 1), c(2, 5, 2), na_rm = "x")
#> Error in if (na_rm) {: argument is not interpretable as logical
yardstick::rmse_vec(c(1, 3, NA), c(2, 5, 2), na_rm = c(TRUE, FALSE))
#> Error in if (na_rm) {: the condition has length > 1

^{Created on 2022-12-10 by the reprex package (v2.0.1)}

Would it be possible to check the type of na_rm before the if() statement?

EmilHvitfeldt commented 1 year ago

This seems like a reasonable thing to do. Should properly live in the *_metric_summarizer() functions as a check_na_rm() call

EmilHvitfeldt commented 1 year ago

tidymodels / yardstick

Check type of `na_rm`? #349

Feature