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.

Sign up for GitHub

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

Add warning message when scaling steps return NaN #1252

Merged
merged 9 commits into from
Nov 16, 2023
Merged

Add warning message when scaling steps return NaN #1252

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
c211023
add warning message when scaling returns NaN
mastoffel Oct 20, 2023
512f3fc
change expect_warning to expect_snapshot
mastoffel Nov 4, 2023
520dbce
change step_scale NaN warning to cli_warn()
mastoffel Nov 4, 2023
d734011
add NaN warning to step_range and step_center
mastoffel Nov 4, 2023
28e2942
update news
EmilHvitfeldt Nov 16, 2023
f6e95f8
fix spacing
EmilHvitfeldt Nov 16, 2023
ed80752
add a couple of expert_snapshot()
EmilHvitfeldt Nov 16, 2023
a716468
update snapshots
EmilHvitfeldt Nov 16, 2023
c469552
also test step_normalize()
EmilHvitfeldt Nov 16, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions R/normalize.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,21 @@ sd_check <- function(x) {
)
x[zero_sd] <- 1
}

na_sd <- which(is.na(x))
if (length(na_sd) > 0) {
glue_cols <- glue::glue_collapse(
glue("`{names(na_sd)}`"), sep = ", ", last = " and "
)
rlang::warn(
glue(
"Column(s) {glue_cols} returned NaN, because variance cannot be ",
"calculated and scaling cannot be used. ",
"Consider avoiding `Inf` or `-Inf` values and/or setting `na_rm=TRUE` ",
"before normalizing."
)
)
mastoffel marked this conversation as resolved.
Show resolved Hide resolved
}
x
}

Expand Down
9 changes: 9 additions & 0 deletions tests/testthat/test-scale.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,9 +88,11 @@ test_that("na_rm argument works for step_scale", {
mtcars_na <- mtcars
mtcars_na[1, 1:4] <- NA

expect_warning({
mastoffel marked this conversation as resolved.
Show resolved Hide resolved
rec_no_na_rm <- recipe(~., data = mtcars_na) %>%
step_scale(all_predictors(), na_rm = FALSE) %>%
prep()
})

rec_na_rm <- recipe(~., data = mtcars_na) %>%
step_scale(all_predictors(), na_rm = TRUE) %>%
Expand All @@ -115,6 +117,13 @@ test_that("warns on zv",{
expect_snapshot(prep(rec1))
})

test_that("warns when NaN is returned",{
rec1 <- rec %>%
step_log(sulfur) %>%
step_scale(sulfur)
expect_warning(prep(rec1))
mastoffel marked this conversation as resolved.
Show resolved Hide resolved
})

test_that("scaling with case weights", {
mtcars_freq <- mtcars
mtcars_freq$cyl <- frequency_weights(mtcars_freq$cyl)
Expand Down