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

tibble() - Constructing a tibble with NULL evaluating expression arguments inconsistent with constructing a tibble with NULL valued arguments. #895

Closed
elmstedt opened this issue Jun 13, 2021 · 3 comments · Fixed by #902
Labels
bug an unexpected problem or unintended behavior
Milestone

Comments

@elmstedt
Copy link

While debugging a Redditor's question I came across an issue I've distilled here.

library(tibble)
tib_works <- tibble(x = NULL, y = NULL, z = NULL)
tib_fails <- tibble(x = c(), y = c(), z = c())

str(tib_works)
#> tibble [0 x 0] (S3: tbl_df/tbl/data.frame)
#>  Named list()

str(tib_fails)
#> tibble [0 x 3] (S3: tbl_df/tbl/data.frame)
#>  $ x: NULL
#>  $ y: NULL
#>  $ z: NULL

tib_works
#> # A tibble: 0 x 0
tib_fails
#> Error: Internal error in `vec_slice_impl()`: Unexpected `NULL`.

rlang::last_trace()
<error/rlang_error>
Internal error in `vec_slice_impl()`: Unexpected `NULL`.
Backtrace:
     x
  1. +-(function (x, ...) ...
  2. \-pillar:::print.tbl(x)
  3.   +-base::writeLines(format(x, width = width, ..., n = n, n_extra = n_extra))
  4.   +-base::format(x, width = width, ..., n = n, n_extra = n_extra)
  5.   \-pillar:::format.tbl(x, width = width, ..., n = n, n_extra = n_extra)
  6.     \-pillar::tbl_format_setup(x, width = width, ..., n = n, max_extra_cols = n_extra)
  7.       +-pillar:::tbl_format_setup_(x, width, ..., n = n, max_extra_cols = max_extra_cols)
  8.       \-pillar:::tbl_format_setup.tbl(x, width, ..., n = n, max_extra_cols = max_extra_cols)
  9.         \-pillar:::df_head(x, n)
 10.           \-pillar:::vec_head(as.data.frame(x), n)
 11.             \-vctrs::vec_slice(x, seq_len(n))
 12.               \-(function () ...

df1 <- data.frame(x = NULL, y = NULL, z = NULL)
df2 <- data.frame(x = c(), y = c(), z = c())
identical(df1, df2)
#> [1] TRUE

Created on 2021-06-13 by the reprex package (v2.0.0)

Setting aside the larger issues that this is, to say the least, poor form, not doing what they intend, and would invariably lead to other issues immediately thereafter, is this the intended and desired behaviour of tibble()?

A clear analogy here would be to treat this like a patient telling their doctor "it hurts when I do this," and respond, "well, stop doing that! But, in the interest of helping novices should tibble() here:

  1. Throw an error.
  2. Throw a warning.
  3. Behave similarly to data.frame() and produce identical objects in both cases.
  4. Do nothing differently, tibble() is working as intended. This is a PEBCAK situation.

Alternately, should an error be thrown further up the chain somewhere in pillar, perhaps in pillar:::format.tbl() where an error message more relevant to the user's input could be generated?

@krlmlr
Copy link
Member

krlmlr commented Jun 24, 2021

Thanks, confirmed. We need to fix that. Smaller reprex:

dput(tibble::tibble(x = c(), y = c(), z = c()))
#> structure(list(x = NULL, y = NULL, z = NULL), row.names = integer(0), class = c("tbl_df", 
#> "tbl", "data.frame"))

Created on 2021-06-24 by the reprex package (v2.0.0)

@krlmlr krlmlr added the bug an unexpected problem or unintended behavior label Jun 24, 2021
@krlmlr krlmlr added this to the 3.1.2 milestone Jun 24, 2021
@DavisVaughan
Copy link
Member

DavisVaughan commented Jul 1, 2021

I think tibble:::tibble_quos() needs to compact() away any NULLs that appeared after the eval_tidy() loop (and compact the column names as required). It already seems to skip over them in the loop if they appear after calling eval_tidy(), but they are never removed.

That will catch any NULL values that were "computed", i.e. like c().

If you do that, you might also be able to remove the is.null <- map_lgl(xs, quo_is_null) bit from tibble(), and instead allow that post-loop check to remove both "computed" and explicit NULL columns.

@krlmlr krlmlr modified the milestones: 3.1.2, 3.1.3 Jul 17, 2021
krlmlr added a commit that referenced this issue Jul 17, 2021
- `tibble()` and `tibble_row()` ignore all columns that evaluate to `NULL`, not only those where a verbatim `NULL` is passed (#895, #900).
@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 Jul 18, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug an unexpected problem or unintended behavior
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants