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

as_tibble inconsistencies #447

Closed
hadley opened this issue Aug 3, 2018 · 14 comments · Fixed by #728
Closed

as_tibble inconsistencies #447

hadley opened this issue Aug 3, 2018 · 14 comments · Fixed by #728
Labels
vctrs ↗️ Requires vctrs package

Comments

@hadley
Copy link
Member

hadley commented Aug 3, 2018

library(tibble)

# rows vs columns
as_tibble(c(x = 1, y = 2))
#> # A tibble: 2 x 1
#>   value
#> * <dbl>
#> 1     1
#> 2     2
as_tibble(list(x = 1, y = 2))
#> # A tibble: 1 x 2
#>       x     y
#>   <dbl> <dbl>
#> 1     1     2

# fills in names
as_tibble(c(1, 2))
#> # A tibble: 2 x 1
#>   value
#>   <dbl>
#> 1     1
#> 2     2
as_tibble(list(1, 2))
#> Error: Columns 1, 2 must be named.

# different scheme for missing names
as_tibble(t(1:2))
#> # A tibble: 1 x 2
#>      V1    V2
#>   <int> <int>
#> 1     1     2

I'd recommend that as_tibble() always treats inputs as columns as if you want rows, you can use enframe(). Related to #205

@hadley
Copy link
Member Author

hadley commented Aug 3, 2018

Alternatively, we could soft-deprecate as_tibble() and enframe() in favour of something with symmetric names, e.g. as_tibble_row(), as_tibble_col()

@hadley
Copy link
Member Author

hadley commented Aug 3, 2018

cc @jennybc

@krlmlr
Copy link
Member

krlmlr commented Aug 5, 2018

Current as_tibble.default() forwards to as.data.frame(stringsAsFactors = FALSE), to handle inputs we didn't anticipate. It's also the primary conversion function for data frames -> tibbles, I'd rather avoid renaming it.

We could add a special check for vectors/matrices and e.g. show a message that points to a method that won't print that message. For vectors where we remove the names we could use enframe(name = NULL) (#449). Auto-naming could be made consistent with repair_names() .

@hadley
Copy link
Member Author

hadley commented Aug 15, 2018

I think the invariant of as_tibble() is that ncol(tibble(x)) == length(x) (assuming the object doesn't have dimensions)

@hadley
Copy link
Member Author

hadley commented Aug 15, 2018

For names, we need to apply the same treatment to #445.

@krlmlr
Copy link
Member

krlmlr commented Aug 19, 2018

Do we need to distinguish between plain and classed atomic vectors (e.g. from hms)?

@krlmlr krlmlr closed this as completed in 2603b6b Aug 20, 2018
@krlmlr
Copy link
Member

krlmlr commented Aug 20, 2018

In master now, let's discuss improvements in new issues.

krlmlr added a commit that referenced this issue Nov 14, 2018
@krlmlr krlmlr reopened this Nov 14, 2018
@krlmlr krlmlr added the vctrs ↗️ Requires vctrs package label Jan 29, 2019
@krlmlr
Copy link
Member

krlmlr commented Aug 23, 2019

Suggesting to deprecate as_tibble.list() (in favor of new_tibble()) and not add new behaviors.

@hadley
Copy link
Member Author

hadley commented Aug 23, 2019

Both should be soft-deprecations

earowang referenced this issue in tidyverts/fabletools Sep 10, 2019
@krlmlr
Copy link
Member

krlmlr commented Oct 26, 2019

Does new_tibble() need a .name_repair or name_repair argument? We have tons of tests that test as_tibble(list(...)) .

@lionel-
Copy link
Member

lionel- commented Nov 4, 2019

Something feels wrong about this change, because I see new_ functions as developer-oriented, whereas as_ functions are user-oriented. Maybe users should never have to use new_ functions, and should use either variadic constructors (possibly with splicing), or conversion functions. This rule would ensure that it is easy for someone to figure out the prefix, and would avoid apparent (or actual) inconsistencies in the way we use prefixes.

@krlmlr
Copy link
Member

krlmlr commented Nov 4, 2019

We wanted to do as_tibble_row() and as_tibble_col() originally. Would that still be a good solution?

@krlmlr
Copy link
Member

krlmlr commented Dec 20, 2019

Agreed to retire as_tibble.list(). Need to add retirement badge to as_tibble() docs and implement as_tibble_row() and as_tibble_col().

@github-actions
Copy link
Contributor

This old thread has been automatically locked. If you think you have found something related to this, please open a new issue and link to this old issue if necessary.

@github-actions github-actions bot locked and limited conversation to collaborators Mar 16, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
vctrs ↗️ Requires vctrs package
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants