Support new_tibble(nrow = NULL)

tidyverse · Jul 29, 2021 · eace124 · eace124
1 parent 6adac52
commit eace124
Show file tree

Hide file tree

Showing 8 changed files with 39 additions and 47 deletions.
diff --git a/R/new.R b/R/new.R
@@ -7,16 +7,19 @@
 #'
 #' `new_tibble()` creates a new object as a subclass of `tbl_df`, `tbl` and `data.frame`.
 #' This function is optimized for performance, checks are reduced to a minimum.
+#' See [vctrs::new_data_frame()] for details.
 #'
 #' @param x A tibble-like object.
 #' @param ... Name-value pairs of additional attributes.
-#' @param nrow The number of rows, required.
+#' @param nrow The number of rows, inferred from `x` if omitted.
 #' @param class Subclasses to assign to the new object, default: none.
 #' @param subclass Deprecated, retained for compatibility. Please use the `class` argument.
 #'
 #' @seealso
 #' [tibble()] and [as_tibble()] for ways to construct a tibble
-#' with recycling of scalars and automatic name repair.
+#' with recycling of scalars and automatic name repair,
+#' and [vctrs::df_list()] and [vctrs::new_data_frame()]
+#' for lower-level implementations.
 #'
 #' @export
 #' @examples
@@ -28,7 +31,7 @@
 #'
 #' # The length of all columns must be compatible with the nrow argument:
 #' try(validate_tibble(new_tibble(list(a = 1:3, b = 4:6), nrow = 2)))
-new_tibble <- function(x, ..., nrow, class = NULL, subclass = NULL) {
+new_tibble <- function(x, ..., nrow = NULL, class = NULL, subclass = NULL) {
   # For compatibility with tibble < 2.0.0
   if (is.null(class) && !is.null(subclass)) {
     deprecate_soft("2.0.0", "tibble::new_tibble(subclass = )", "new_tibble(class = )")
@@ -44,26 +47,13 @@ new_tibble <- function(x, ..., nrow, class = NULL, subclass = NULL) {
     cnd_signal(error_new_tibble_must_be_list())
   }
 
-  #' An `nrow` argument is required.
-  if (missing(nrow)) {
-    cnd <- error_new_tibble_needs_nrow()
-    if (length(x) >= 1) {
-      deprecate_soft("2.0.0", "tibble::new_tibble(nrow = 'can\\'t be missing')",
-        details = cnd$message)
-      nrow <- vec_size(x[[1]])
-    } else {
-      cnd_signal(cnd)
-    }
-  }
-  #' This should be an integer of length 1,
-  #' and every element of the list `x` should have [vctrs::vec_size()]
+  #' The `nrow` argument may be omitted as of tibble 3.1.4.
+  #' If present, every element of the list `x` should have [vctrs::vec_size()]
   #' equal to this value.
   #' (But this is not checked by the constructor).
   #' This takes the place of the "row.names" attribute in a data frame.
-  if (is_integerish(nrow, 1)) {
+  if (!is.null(nrow) && is_integerish(nrow, 1)) {
     nrow <- as.integer(nrow)
-  } else {
-    cnd_signal(error_new_tibble_needs_nrow())
   }
 
   args <- attributes(x)
@@ -168,7 +158,3 @@ tibble_class_no_data_frame <- c("tbl_df", "tbl")
 error_new_tibble_must_be_list <- function() {
   tibble_error("`x` must be a list.")
 }
-
-error_new_tibble_needs_nrow <- function() {
-  tibble_error("`x` must be a scalar integer.")
-}
diff --git a/man/new_tibble.Rd b/man/new_tibble.Rd
diff --git a/tests/testthat/_snaps/msg.md b/tests/testthat/_snaps/msg.md
@@ -186,11 +186,6 @@
     Output
       <error/tibble_error_new_tibble_must_be_list>
       `x` must be a list.
-    Code
-      error_new_tibble_needs_nrow()
-    Output
-      <error/tibble_error_new_tibble_needs_nrow>
-      `x` must be a scalar integer.
     Code
       # # rownames
       error_already_has_rownames()

diff --git a/tests/testthat/_snaps/new.md b/tests/testthat/_snaps/new.md
@@ -6,9 +6,6 @@
       `x` must be a list.
     Code
       new_tibble(as.list(1:3))
-    Warning <lifecycle_warning_deprecated>
-      The `nrow` argument of `new_tibble()` can't be missing as of tibble 2.0.0.
-      `x` must be a scalar integer.
     Error <tibble_error_names_must_be_non_null>
       `names` must not be `NULL`.
 
diff --git a/tests/testthat/helper-zzz.R b/tests/testthat/helper-zzz.R
@@ -85,6 +85,11 @@ skip_enh_tibble_null <- function() {
   skip_legacy()
 }
 
+skip_enh_new_tibble_nrow_null <- function() {
+  # ENH: new_tibble(nrow = NULL), #781
+  skip_legacy()
+}
+
 skip_enh_empty_tribble_unspecified <- function() {
   # ENH: zero-row tribbles create unspecified columns
   skip_legacy()

diff --git a/tests/testthat/test-msg.R b/tests/testthat/test-msg.R
@@ -59,8 +59,6 @@ test_that("output test", {
     "# new"
     error_new_tibble_must_be_list()
 
-    error_new_tibble_needs_nrow()
-
     "# rownames"
     error_already_has_rownames()
 

diff --git a/tests/testthat/test-new.R b/tests/testthat/test-new.R
@@ -84,6 +84,11 @@ test_that("new_tibble() allows setting names through `...`", {
   )
 })
 
+test_that("new_tibble() supports missing `nrow` (#781)", {
+  expect_identical(new_tibble(list()), tibble())
+  expect_identical(new_tibble(list(a = 1:3)), tibble(a = 1:3))
+})
+
 test_that("new_tibble checks", {
   scoped_lifecycle_errors()
 
@@ -94,14 +99,6 @@ test_that("new_tibble checks", {
     new_tibble(1:3, nrow = 1),
     error_new_tibble_must_be_list()
   )
-  expect_error(
-    new_tibble(list(a = 1)),
-    class = get_defunct_error_class()
-  )
-  expect_tibble_error(
-    new_tibble(list(1), nrow = NULL),
-    error_new_tibble_needs_nrow()
-  )
   expect_tibble_error(
     new_tibble(list(1), nrow = 1),
     error_names_must_be_non_null()

diff --git a/tests/testthat/test-zzz-tbl-df.R b/tests/testthat/test-zzz-tbl-df.R
@@ -422,6 +422,13 @@ test_that("new_tibble checks", {
     error_new_tibble_must_be_list(),
     fixed = TRUE
   )
+})
+
+test_that("new_tibble checks (2)", {
+  skip_enh_new_tibble_nrow_null()
+
+  scoped_lifecycle_errors()
+
   expect_legacy_error(
     new_tibble(list(a = 1)),
     class = get_defunct_error_class(),
@@ -433,6 +440,11 @@ test_that("new_tibble checks", {
     error_new_tibble_needs_nrow(),
     fixed = TRUE
   )
+})
+
+test_that("new_tibble checks (3)", {
+  scoped_lifecycle_errors()
+
   expect_legacy_error(
     new_tibble(list(1), nrow = 1),
     error_names_must_be_non_null(repair_hint = FALSE),