yardstick icon indicating copy to clipboard operation
yardstick copied to clipboard

Check type of `na_rm`?

Open mikemahoney218 opened this issue 2 years ago • 2 comments

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?

mikemahoney218 avatar Dec 10 '22 15:12 mikemahoney218

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

EmilHvitfeldt avatar Feb 08 '23 00:02 EmilHvitfeldt

related to https://github.com/tidymodels/yardstick/issues/419

EmilHvitfeldt avatar Apr 18 '23 15:04 EmilHvitfeldt