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

Devel vctrs::vec_rbind does not remove column names in list columns #1007

Closed
colearendt opened this issue Apr 13, 2020 · 6 comments
Closed

Comments

@colearendt
Copy link

This is breaking behavior for https://github.com/colearendt/tidyjson, so wanted to report it. It is not clear to me whether this is an intentional or unintentional change in behavior. It seems marginally related to #999 and #966.

As I think about this behavior, it seems almost desirable to keep the names - it was not clear to me why names were dropped in the first place (but we do depend on it in a few places, for better or worse. Others may as well).

I originally noticed the behavior in tidyr::unnest() and traced it here:

original behavior

Notice that chr "b" is not named

packageVersion("vctrs")
#> [1] '0.2.4'
hm <- tibble::tibble(col = list(a = "b"))
str(vctrs::vec_rbind(hm, .ptype = NULL))
#> tibble [1 × 1] (S3: tbl_df/tbl/data.frame)
#>  $ col:List of 1
#>   ..$ : chr "b"

Created on 2020-04-13 by the reprex package (v0.3.0)

devel behavior

Notice that chr "b" retains its name

packageVersion("vctrs")
#> [1] '0.2.99.9011'
hm <- tibble::tibble(col = list(a = "b"))
str(vctrs::vec_rbind(hm, .ptype = NULL))
#> tibble [1 × 1] (S3: tbl_df/tbl/data.frame)
#>  $ col:List of 1
#>   ..$ a: chr "b"

Created on 2020-04-13 by the reprex package (v0.3.0)

@lionel-
Copy link
Member

lionel- commented Apr 13, 2020

This is intended behaviour, the inner names are now correctly kept.

@colearendt
Copy link
Author

Thanks! Do you know why the legacy behavior existed? Is this the behavior that rbind has or something?

@hadley
Copy link
Member

hadley commented Apr 13, 2020

It's what data.frame does:

data.frame(x = 1:2, y = c(a = 1, b = 2))$y
#> [1] 1 2

Created on 2020-04-13 by the reprex package (v0.3.0)

(But I now think that's tangled up with how data frames convert the names of the first column to row names)

@lionel-
Copy link
Member

lionel- commented Apr 13, 2020

It was because we used vec_assign() (our equivalent of [<-) to fill a pre-initalised vector, and nor vec-assign neither [<- copy over names / inner names. Now our internal version of vec-assign has a parameter to allow copying the names.

@colearendt
Copy link
Author

Awesome, that background is super helpful, thanks! Is it worth noting this as a change / breaking change? It was a hard one to trace down, although perhaps it is unusual for someone to depend on something not having names.

@lionel-
Copy link
Member

lionel- commented Apr 16, 2020

The NEWS item is now in a breaking change section, thanks for the report!

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

No branches or pull requests

3 participants