-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Implement replace_when()
#7728
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
Implement replace_when()
#7728
Conversation
NEWS.md
Outdated
| * `case_when()` is now part of a family of 4 related functions, 3 of which are new: | ||
|
|
||
| * Use `case_when()` to create a new vector based on logical conditions. | ||
| * Use `replace_when()` to update an existing vector based on logical conditions. | ||
| * Use `recode_values()` to create a new vector by mapping old values to new values. | ||
| * Use `replace_values()` to update an existing vector by mapping old values to new values. | ||
|
|
||
| We are particularly excited about `recode_values()`, which allows you to easily incorporate a lookup table, and serves as a more holistic replacement for both `case_match()` and `recode()`. | ||
|
|
||
| This work is a result of [Tidyup 7: Recoding and replacing values in the tidyverse](https://github.com/tidyverse/tidyups/blob/main/007-tidyverse-recoding-and-replacing.md), with a lot of great [feedback](https://github.com/tidyverse/tidyups/pull/29) from the community. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Preemptive NEWS bullet for the family of 4, the other 2 are coming up next
| @@ -1,36 +1,83 @@ | |||
| #' A general vectorised if-else | |||
| #' | |||
| #' @description | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have fully refreshed the case_when() docs, so it's probably worth looking through them holistically
| test_that("replace_when() does not recycle LHS values", { | ||
| # Unlike `case_when()` we get to do this right! | ||
| x <- c(1, 2, 3) | ||
|
|
||
| expect_snapshot(error = TRUE, { | ||
| replace_when(x, TRUE ~ 0) | ||
| }) | ||
| }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Notable test!
| test_that("replace_when() does not allow named `...`", { | ||
| # Purposefully stricter than `case_when()` | ||
| expect_snapshot(error = TRUE, { | ||
| replace_when(1, foo = TRUE ~ 2) | ||
| }) | ||
| }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Notable test!
| test_that("replace_when() is a no-op with zero conditions", { | ||
| # Unlike `case_when()`, where when zero conditions are supplied | ||
| # we don't know what kind of vector to build (and we refuse to | ||
| # build an `unspecified` vector, unlike `vec_case_when()`) | ||
| expect_identical(replace_when(1), 1) | ||
| expect_identical(replace_when(1, NULL), 1) | ||
| }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Notable test! replace_when(1) is well defined, while case_when() (that exact call, with no inputs) is not
(Well, case_when() could technically return unspecified(), or incorporate .ptype, .size, and .default if specified, all of which vec_case_when() does, but for the average user it's better to error, and historically that is what case_when() has always done here)
c53c097 to
eda7ccd
Compare
R/case-when.R
Outdated
| #' For `case_when()`: | ||
| #' | ||
| #' The RHS inputs will be coerced to their common type. | ||
| #' - The LHS inputs must be logical vectors. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For both case_when() and replace_when(), the LHS must be a logical vector. For backward compatibility, case_when() recycles scalar values to their common size, but we no longer recommend this. For replace_when(), the LHS must be the same length as x.
It feels like overall this documentation gets too into the weeds too quickly. We should define the size/type guarantees somewhere, but I feel like it might be better in a details section, rather than in the argument docs.
Part of tidyverse/tidyups#29
Closes #4050 (our second oldest issue!)