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

Draft list_flatten() #1214

Closed
wants to merge 4 commits into from
Closed

Draft list_flatten() #1214

wants to merge 4 commits into from

Conversation

lionel-
Copy link
Member

@lionel- lionel- commented Aug 10, 2020

No description provided.

@lionel- lionel- marked this pull request as draft August 10, 2020 09:34
Allows implementing e.g. `rlang::flatten_int()`
@lionel-
Copy link
Member Author

lionel- commented Aug 10, 2020

Added a ptype argument that can be an atomic vector. In that case, the list is first flattened and then assembled into a vector of the requested type.

x <- list(1, list(2:3))

list_flatten(x, ptype = integer())
#> [1] 1 2 3

These are the same semantics as in rlang:

rlang::flatten_int(x)
#> [1] 1 2 3

Conceptually, rlang atomic flattens wrap list_flatten(), whereas purrr ones wrap list_unchop().

@lionel-
Copy link
Member Author

lionel- commented Aug 10, 2020

I completely forgot that purrr converts atomic vectors to lists before flattening:

purrr::flatten(list(1, 2:3))
#> [[1]]
#> [1] 1
#>
#> [[2]]
#> [1] 2
#>
#> [[3]]
#> [1] 3

This is not how list_flatten() currently works. Instead of chopping atomic vectors, it wraps them:

list_flatten(list(1, 2:3))
#> [[1]]
#> [1] 1
#>
#> [[2]]
#> [1] 2 3

Interestingly, switching to these semantics makes the list and non-list paths opposite of each other. For list flattening, all non-list elements are chopped. For atomic flattening, all list elements are unchopped:

  if (vec_is_list(ptype)) {
    # Chop all non-list elements so all the elements to concatenate are lists
    out <- map_if(x, negate(vec_is_list), vec_chop)
  } else {
    # Unchop all list elements so all the elements to concatenate are of type `ptype`
    out <- map_if(x, vec_is_list, vec_unchop, ptype = ptype, name_spec = name_spec)
  }

One disadvantage of these semantics is that it is an error to flatten lists that contain scalar elements:

# CRAN
flatten(list(function() NULL))
#> Error: Element 1 of `.x` must be a vector, not a function

I'm not sure what to think. On the one hand it seems to make sense that you could iteratively flatten a list until it no longer has list elements (is completely flat). In that case, non-list elements should be preserved as is. On the other hand, flatten() should normally be called after a splitting operation, and the purrr semantics makes it possible to deal with different kinds of splitting operations: ones that return atomic vectors, and ones that return lists.

I may be coming around to the current behaviour in purrr.

@DavisVaughan
Copy link
Member

DavisVaughan commented Aug 10, 2020

This feels a little unexpected to me, I think I like how list_flatten() currently works. Not erroring on scalar elements seems useful to me.

purrr::flatten(list(1, 2:3))
#> [[1]]
#> [1] 1
#>
#> [[2]]
#> [1] 2
#>
#> [[3]]
#> [1] 3

This feels sort of like a list_chop()

@lionel-
Copy link
Member Author

lionel- commented Aug 10, 2020

I keep being surprised about how hard flatten() is 😅

@lionel-
Copy link
Member Author

lionel- commented Sep 8, 2022

Closing in favour of tidyverse/purrr#912

@lionel- lionel- closed this Sep 8, 2022
@lionel- lionel- deleted the add-flatten branch September 8, 2022 14:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants