add ignore_step function #1324
add ignore_step function #1324
Conversation
There was a problem hiding this comment.
I would not use "ignore" in the name here because that implies the step(s) in question are still part of the recipe, they just don't get used -- but this does remove them. I think remove_step() would be the cleanest but if we want to avoid "remove" because of step_rm(), we could go with "undo" or "delete".
| if (n_steps == 0) { | ||
| cli::cli_abort("{.arg x} doesn't contain any steps to remove.") | ||
| } | ||
|
|
||
| arg <- rlang::check_exclusive(number, id) |
There was a problem hiding this comment.
We could allow ignore_step(<recipes without steps>) and just have it do nothing. If we don't want that, I would swap these two checks. To me, it's more obvious then that we don't allow that because we always require you to provide what to remove (before we tell you that there is nothing to remove).
| #' | ||
| #' ignore_step(rec, id = "PCA") | ||
| #' @export | ||
| ignore_step <- function(x, number, id) { |
There was a problem hiding this comment.
I would give number and id some default values since only one of the two is required. tidy() uses NA.
| expect_equal( | ||
| ignore_attr = TRUE, | ||
| ignore_step(rec1234, number = 1), | ||
| rec234 | ||
| ) |
There was a problem hiding this comment.
I'm surprised by the argument order here, why do you put ignore_attr up front?
| #' `ignore_step` will return a recipe without steps specified by the `number` or | ||
| #' `id` argument. | ||
| #' | ||
| #' @param x A `recipe` object. |
There was a problem hiding this comment.
| #' @param x A `recipe` object. | |
| #' @param x An untrained `recipe` object. |
| #' @param number An integer vector, Denoting the positions of the steps that | ||
| #' should be removed. | ||
| #' @param id A character string. Denoting the `id` of the steps that should be | ||
| #' removed. |
There was a problem hiding this comment.
| #' @param number An integer vector, Denoting the positions of the steps that | |
| #' should be removed. | |
| #' @param id A character string. Denoting the `id` of the steps that should be | |
| #' removed. | |
| #' @param number An integer vector denoting the positions of the steps that | |
| #' should be removed. | |
| #' @param id A character string denoting the `id` of the steps that should be | |
| #' removed. |
|
We are sitting on this for another week. Brief summary of meeting discussion:
|
|
we are sitting on this at least another week |
to close #887
Adds
ignore_step(), with a similar interface astidy()with respect to how we select, either usingnumberorid.I'm not in love with the name, so suggestions are welcome.