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.

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

.ragged default in modify_depth is not correcly evaluated #530

Closed
cderv opened this issue Aug 12, 2018 · 0 comments
Closed

.ragged default in modify_depth is not correcly evaluated #530

cderv opened this issue Aug 12, 2018 · 0 comments

Comments

@cderv
Copy link
Contributor

@cderv cderv commented Aug 12, 2018

library(purrr)
x1 <- list(
  list(1),
  list(list(2))
)
# it works as expected
modify_depth(x1, .depth = -1, ~ . + 1, .ragged = TRUE) %>% str()
#> List of 2
#>  $ :List of 1
#>   ..$ : num 2
#>  $ :List of 1
#>   ..$ :List of 1
#>   .. ..$ : num 3

# .ragged should also be TRUE by default here because .depth is negative
modify_depth(x1, .depth = -1, ~ . + 1)
#> Error: List not deep enough

We have an error because value of .ragged is FALSE, because .depth is modified
internally before .ragged is evaluated.

purrr/R/modify.R

Lines 203 to 212 in 977f1fd

modify_depth.default <- function(.x, .depth, .f, ..., .ragged = .depth < 0) {
stopifnot(is_integerish(.depth, n = 1))
if (.depth < 0) {
.depth <- vec_depth(.x) + .depth
}
.f <- as_mapper(.f, ...)
modify_depth_rec(.x, .depth, .f, ..., .ragged = .ragged)
}

cderv added a commit to cderv/purrr that referenced this issue Aug 12, 2018
Otherwise .depth is modified and .ragged is not evaluated to the correct value. closes tidyverse#530
@hadley hadley closed this in #531 Sep 22, 2018
hadley added a commit that referenced this issue Sep 22, 2018
* force .ragged evaluation in modify_depth

Otherwise .depth is modified and .ragged is not evaluated to the correct value. closes #530

* add NEWS bullet
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

1 participant