Check for POSIXct when creating data_frames.

Fixes tidyverse#813

R/dataframe.R

@@ -30,6 +30,13 @@
 #'
 #' # With the SE version, you give it a list of formulas/expressions
 #' data_frame_(list(x = ~1:10, y = quote(x * 2)))
+#'
+#' # data frames can only contain 1d atomic lists and vectors
+#' # and can not contain POSIXlt
+#' \dontrun{
+#' data_frame(x = data_frame(1, 2, 3))
+#' data_frame(y = strptime("2000/01/01", "%x"))
+#' }
 data_frame <- function(...) {
   data_frame_(lazyeval::lazy_dots(...))
 }
@@ -64,9 +71,14 @@ data_frame_ <- function(columns) {
     # Fill by reference
     res <- lazyeval::lazy_eval(columns[[i]], output)
     if (!is_1d(res)) {
-      stop("data_frames can only contain 1d atomic vectors and lists",
+      stop("`", col_names[i], "` is not an 1d atomic vector or list.",
+        call. = FALSE)
+    }
+    if (inherits(res, "POSIXlt")) {
+      stop("`", col_names[i], "` is a `POSIXlt`; use a `POSIXct` instead.",
         call. = FALSE)
     }
+
     output[[i]] <- res
     names(output)[i] <- col_names[[i]]
 
@@ -137,10 +149,18 @@ as_data_frame <- function(x) {
 
   ok <- vapply(x, is_1d, logical(1))
   if (any(!ok)) {
-    stop("data_frames can only contain 1d atomic vectors and lists",
+    bad <- names(x)[!ok]
+    stop(paste0("`", bad, "`", collapse = ","), ": not 1d atomic vectors or lists.",
       call. = FALSE)
   }
 
+  posixlt <- vapply(x, inherits, "POSIXlt", FUN.VALUE = logical(1))
+  if (any(posixlt)) {
+    bad <- names(x)[posixlt]
+    stop(paste0("`", bad, "`", collapse = ","), ": are `POSIXlt`; ",
+      "use `POSIXct` instead", call. = FALSE)
+  }
+
   n <- unique(vapply(x, NROW, integer(1)))
   if (length(n) != 1) {
     stop("Columns are not all same length", call. = FALSE)

R/utils.r

@@ -137,8 +137,10 @@ succeeds <- function(x, quiet = FALSE) {
   })
 }
 
+# is.atomic() is TRUE for atomic vectors AND NULL!
+is_atomic <- function(x) is.atomic(x) && !is.null(x)
+
 is_1d <- function(x) {
-  # is.atomic() is TRUE for atomic vectors AND NULL!
   # dimension check is for matrices and data.frames
-  ((is.atomic(x) && !is.null(x)) || is.list(x)) && length(dim(x)) <= 1
+  (is_atomic(x) || is.list(x)) && length(dim(x)) <= 1
 }

tests/testthat/test-data_frame.R

@@ -15,13 +15,18 @@ test_that("data_frame returns correct number of rows with all combinatinos", {
 })
 
 test_that("can't make data_frame containing data.frame or array", {
-  expect_error(data_frame(mtcars), "only contain 1d atomic vectors and lists")
-  expect_error(data_frame(diag(5)), "only contain 1d atomic vectors and lists")
+  expect_error(data_frame(mtcars), "`mtcars` is not an 1d atomic vector or list")
+  expect_error(data_frame(x = diag(5)), "`x` is not an 1d atomic vector or list")
+})
+
+test_that("can't make data_frame containing a POSIXlt", {
+  lt <- as.POSIXlt(Sys.time())
+  expect_error(data_frame(x = lt), "`x` is a `POSIXlt`")
 })
 
 test_that("null isn't a valid column", {
-  expect_error(data_frame(a = NULL), "only contain 1d atomic vectors and lists")
-  expect_error(as_data_frame(list(a = NULL)), "only contain 1d atomic vectors and lists")
+  expect_error(data_frame(a = NULL), "`a` is not an 1d atomic vector or list")
+  expect_error(as_data_frame(list(a = NULL)), "`a`: not 1d atomic vectors or lists")
 })
 
 # as_data_frame -----------------------------------------------------------
@@ -40,8 +45,12 @@ test_that("columns must be named", {
 })
 
 test_that("can't coerce list data.frame or array", {
-  expect_error(as_data_frame(list(x = mtcars)), "only contain 1d atomic vectors and lists")
-  expect_error(as_data_frame(list(x = diag(5))), "only contain 1d atomic vectors and lists")
+  expect_error(as_data_frame(list(x = mtcars)), "`x`: not 1d atomic vectors or lists")
+  expect_error(as_data_frame(list(x = diag(5))), "`x`: not 1d atomic vectors or lists")
+})
+
+test_that("can't make data_frame containing a POSIXlt", {
+  expect_error(as_data_frame(list(x = strptime("2000/01/01", "%x"))), "`x`: are `POSIXlt`")
 })
 
 test_that("Zero column list makes 0 x 0 tbl_df", {