From 47ced5cf07b9fcdd41647e824cc4e24df46f3110 Mon Sep 17 00:00:00 2001
From: Hadley Wickham <h.wickham@gmail.com>
Date: Wed, 21 Aug 2024 08:25:44 -0500
Subject: [PATCH] Polishing

---
 R/list-combine.R                   | 28 +++++++++++++++++-----------
 man/list_c.Rd                      |  8 +++++---
 tests/testthat/test-list-combine.R | 15 ++++++++++++---
 3 files changed, 34 insertions(+), 17 deletions(-)

diff --git a/R/list-combine.R b/R/list-combine.R
index 5acee987..f9c8abf6 100644
--- a/R/list-combine.R
+++ b/R/list-combine.R
@@ -22,8 +22,9 @@
 #'   same size (i.e. number of rows).
 #' @param name_repair One of `"unique"`, `"universal"`, or `"check_unique"`.
 #'   See [vctrs::vec_as_names()] for the meaning of these options.
-#' @param keep_empty  An optional logical. If FALSE (the default), then the empty element is silently ignored;
-#'  if TRUE, then the empty element is kept as an NA`.
+#' @param keep_empty  An optional logical. If `FALSE` (the default), then
+#'   empty (`NULL`) elements are silently ignored; if `TRUE`, then empty
+#'   elements are preserved by converting to `NA`.
 #' @inheritParams rlang::args_dots_empty
 #' @export
 #' @examples
@@ -33,7 +34,7 @@
 #' x2 <- list(
 #'   a = data.frame(x = 1:2),
 #'   b = data.frame(y = "a"),
-#'   c = NULL)
+#'   c = NULL
 #' )
 #' list_rbind(x2)
 #' list_rbind(x2, names_to = "id")
@@ -41,12 +42,12 @@
 #' list_rbind(unname(x2), names_to = "id")
 #' list_cbind(x2)
 #' list_cbind(x2, keep_empty = TRUE)
-#'
 list_c <- function(x, ..., ptype = NULL, keep_empty = FALSE) {
   vec_check_list(x)
   check_dots_empty()
-
-  if(keep_empty) x <- convert_empty_element_to_NA(x)
+  if (keep_empty) {
+    x <- convert_null_to_NA(x)
+  }
 
   # For `list_c()`, we don't expose `list_unchop()`'s `name_spec` arg,
   # and instead strip outer names to avoid collisions with inner names
@@ -70,7 +71,9 @@ list_cbind <- function(
   ) {
   check_list_of_data_frames(x)
   check_dots_empty()
-  if(keep_empty) x <- convert_empty_element_to_NA(x)
+  if (keep_empty) {
+    x <- convert_null_to_NA(x)
+  }
 
   vec_cbind(!!!x, .name_repair = name_repair, .size = size, .error_call = current_env())
 }
@@ -80,7 +83,9 @@ list_cbind <- function(
 list_rbind <- function(x, ..., names_to = rlang::zap(), ptype = NULL, keep_empty = FALSE) {
   check_list_of_data_frames(x)
   check_dots_empty()
-  if(keep_empty) x <- convert_empty_element_to_NA(x)
+  if (keep_empty) {
+    x <- convert_null_to_NA(x)
+  }
 
   vec_rbind(!!!x, .names_to = names_to, .ptype = ptype, .error_call = current_env())
 }
@@ -106,7 +111,8 @@ check_list_of_data_frames <- function(x, error_call = caller_env()) {
   )
 }
 
-## used to convert empty elements into NA for list_binding functions
-convert_empty_element_to_NA = function(x) {
-  map(x, function(x) if(vctrs::vec_is_empty(x)) NA else x)
+convert_null_to_NA <- function(x) {
+  is_null <- map_lgl(x, is.null)
+  x[is_null] <- list(NA)
+  x
 }
diff --git a/man/list_c.Rd b/man/list_c.Rd
index b7c1f326..d88f57ec 100644
--- a/man/list_c.Rd
+++ b/man/list_c.Rd
@@ -27,7 +27,9 @@ only contain only data frames or \code{NULL}.}
 \item{ptype}{An optional prototype to ensure that the output type is always
 the same.}
 
-\item{keep_empty}{An optional Logical to keep empty elements of a list as NA.}
+\item{keep_empty}{An optional logical. If \code{FALSE} (the default), then
+empty (\code{NULL}) elements are silently ignored; if \code{TRUE}, then empty
+elements are preserved by converting to \code{NA}.}
 
 \item{name_repair}{One of \code{"unique"}, \code{"universal"}, or \code{"check_unique"}.
 See \code{\link[vctrs:vec_as_names]{vctrs::vec_as_names()}} for the meaning of these options.}
@@ -57,12 +59,12 @@ list_c(x1)
 x2 <- list(
   a = data.frame(x = 1:2),
   b = data.frame(y = "a"),
-  c = data.frame(z = NULL)
+  c = NULL
 )
 list_rbind(x2)
 list_rbind(x2, names_to = "id")
 list_rbind(x2, names_to = "id", keep_empty = TRUE)
 list_rbind(unname(x2), names_to = "id")
 list_cbind(x2)
-
+list_cbind(x2, keep_empty = TRUE)
 }
diff --git a/tests/testthat/test-list-combine.R b/tests/testthat/test-list-combine.R
index 054d66f7..86e47008 100644
--- a/tests/testthat/test-list-combine.R
+++ b/tests/testthat/test-list-combine.R
@@ -74,9 +74,18 @@ test_that("NULLs are converted to NA when keep_empty = TRUE", {
   df1 <- data.frame(x = 1)
   df2 <- data.frame(y = 1)
 
-  expect_equal(list_c(list(1, NULL, 2), keep_empty = TRUE), c(1, NA, 2))
-  expect_equal(list_rbind(list(df1, NULL, df1), keep_empty = TRUE), data.frame(x = c(1, NA, 1)))
-  expect_equal(list_cbind(list(df1, z = NULL, df2), keep_empty = TRUE), data.frame(df1, z = NA, df2))
+  expect_equal(
+    list_c(list(1, NULL, 2), keep_empty = TRUE),
+    c(1, NA, 2)
+  )
+  expect_equal(
+    list_rbind(list(df1, NULL, df1), keep_empty = TRUE),
+    data.frame(x = c(1, NA, 1))
+  )
+  expect_equal(
+    list_cbind(list(df1, z = NULL, df2), keep_empty = TRUE),
+    data.frame(df1, z = NA, df2)
+  )
 })
 
 test_that("empty inputs return expected output", {