Merge pull request #17 from jennybc/master

- `as_data_frame(NULL)` is 0-row 0-column data frame (#17, @jennybc).

R/dataframe.R

@@ -76,7 +76,7 @@ lst_ <- function(xs) {
     deparse2 <- function(x) paste(deparse(x$expr, 500L), collapse = "")
     defaults <- vapply(xs[missing_names], deparse2, character(1),
       USE.NAMES = FALSE)
-
+    defaults <- ifelse(defaults == "NULL", "", defaults)
     col_names[missing_names] <- defaults
   }
 
@@ -85,7 +85,10 @@ lst_ <- function(xs) {
   names(output) <- character(n)
 
   for (i in seq_len(n)) {
-    output[[i]] <- lazyeval::lazy_eval(xs[[i]], output)
+    res <- lazyeval::lazy_eval(xs[[i]], output)
+    if (!is.null(res)) {
+      output[[i]] <-  res
+    }
     names(output)[i] <- col_names[[i]]
   }
 
@@ -157,6 +160,9 @@ as_data_frame.data.frame <- function(x, ...) {
 #' @export
 #' @rdname as_data_frame
 as_data_frame.list <- function(x, validate = TRUE, ...) {
+
+  x <- compact(x)
+
   if (length(x) == 0) {
     x <- list()
     class(x) <- c("tbl_df", "tbl", "data.frame")
@@ -189,7 +195,7 @@ as_data_frame.matrix <- function(x, ...) {
 #' @export
 #' @rdname as_data_frame
 as_data_frame.NULL <- function(x, ...) {
-  NULL
+  as_data_frame(list())
 }
 
 #' Convert row names to an explicit variable.

R/utils.r

@@ -1,3 +1,5 @@
+compact <- function(x) Filter(Negate(is.null), x)
+
 names2 <- function(x) {
   names(x) %||% rep("", length(x))
 }

tests/testthat/test-data_frame.R

@@ -19,9 +19,12 @@ test_that("can't make data_frame containing data.frame or array", {
   expect_error(data_frame(diag(5)), "must be a 1d atomic vector or list")
 })
 
-test_that("null isn't a valid column", {
-  expect_error(data_frame(a = NULL))
-  expect_error(as_data_frame(list(a = NULL)), "must be a 1d atomic vector or list")
+test_that("null columns are dropped", {
+  expect_identical(data_frame(a = NULL), data_frame())
+  just_b <- data_frame(a = NULL, b = character())
+  expect_is(just_b, "tbl_df")
+  expect_equal(dim(just_b), c(0L, 1L))
+  expect_identical(attr(just_b, "names"), "b")
 })
 
 test_that("length 1 vectors are recycled", {
@@ -81,6 +84,12 @@ test_that("Zero column list makes 0 x 0 tbl_df", {
   expect_equal(dim(zero), c(0L, 0L))
 })
 
+test_that("NULL makes 0 x 0 tbl_df", {
+  nnnull <- as_data_frame(NULL)
+  expect_is(nnnull, "tbl_df")
+  expect_equal(dim(nnnull), c(0L, 0L))
+})
+
 test_that("add_rownames keeps the tbl classes (#882)", {
   res <- add_rownames( mtcars, "Make&Model" )
   expect_equal( class(res), c("tbl_df","tbl", "data.frame"))
@@ -90,10 +99,6 @@ test_that("as.tbl", {
   expect_identical(as.tbl(data.frame()), data_frame())
 })
 
-test_that("as_data_frame(NULL) is NULL, not error", {
-  expect_null(as_data_frame(NULL))
-})
-
 # Validation --------------------------------------------------------------
 
 test_that("2d object isn't a valid column", {

tests/testthat/test-lst.R

@@ -0,0 +1,13 @@
+context("lst")
+
+test_that("lst handles named and unnamed NULL arguments", {
+  expect_equivalent(lst(NULL), list(NULL)) ## name is "" vs NULL :(
+  expect_identical(lst(a = NULL), list(a = NULL))
+  expect_identical(lst(NULL, b = NULL, c = 1:3),
+                   list(NULL, b = NULL, c = 1:3))
+})
+
+test_that("lst handles internal references", {
+  expect_identical(lst(a = 1, b = a), list(a = 1, b = 1))
+  expect_identical(lst(a = NULL, b = a), list(a = NULL, b = NULL))
+})