first and last not load xts namespace when not needed, first on empty…

… dt returns empty dt, closes #3857, #3858 (#3859)

NEWS.md

@@ -354,6 +354,8 @@
 
 22. Optimized `mean` of `integer` columns no longer warns about a coercion to numeric, [#986](https://github.com/Rdatatable/data.table/issues/986). Thanks @dgrtwo for his [YouTube tutorial at 3:01](https://youtu.be/AmE4LXPQErM?t=175) where the warning occurs.
 
+23. Using `first` and `last` function on `POSIXct` object no longer loads `xts` namespace, [#3857](https://github.com/Rdatatable/data.table/issues/3857). `first` on empty `data.table` returns empty `data.table` now [#3858](https://github.com/Rdatatable/data.table/issues/3858).
+
 
 ### Changes in [v1.12.2](https://github.com/Rdatatable/data.table/milestone/14?closed=1)  (07 Apr 2019)
 

R/last.R

@@ -5,33 +5,37 @@
 # If xts is loaded higher than data.table, xts::last will work but slower.
 last = function(x, n=1L, ...) {
   if (nargs()==1L) {
-    if (is.vector(x)) {
-      if (!length(x)) return(x) else return(x[[length(x)]])  # for vectors, [[ works like [
-    } else if (is.data.frame(x)) return(x[NROW(x),])
-  }
-  if(!requireNamespace("xts", quietly = TRUE)) {
-    tail(x, n=n, ...) # nocov
+    if (is.vector(x) || is.atomic(x)) {
+      if (!length(x)) x else x[[length(x)]]
+    } else if (is.data.frame(x)) {
+      x[NROW(x),]
+    }
+  } else if ("package:xts" %chin% search()) {
+    if (!requireNamespace("xts", quietly=TRUE))
+      stop("internal error, package:xts is on search path but could not be loaded via requireNamespace") # nocov
+    if (isTRUE(getOption("datatable.verbose", FALSE)))
+      cat("last: using xts::last\n")
+    xts::last(x, n=n, ...) # UseMethod("last") doesn't find xts's methods, not sure what I did wrong.
   } else {
-    # fix with suggestion from Joshua, #1347
-    if (!"package:xts" %chin% search()) {
-      tail(x, n=n, ...) # nocov
-    } else xts::last(x, n=n, ...) # UseMethod("last") doesn't find xts's methods, not sure what I did wrong.
+    tail(x, n=n, ...) # nocov
   }
 }
 
 # first(), similar to last(), not sure why this wasn't exported in the first place...
 first = function(x, n=1L, ...) {
   if (nargs()==1L) {
-    if (is.vector(x)) {
-      if (!length(x)) return(x) else return(x[[1L]])
-    } else if (is.data.frame(x)) return(x[1L,])
-  }
-  if(!requireNamespace("xts", quietly = TRUE)) {
-    head(x, n=n, ...) # nocov
+    if (is.vector(x) || is.atomic(x)) {
+      if (!length(x)) x else x[[1L]]
+    } else if (is.data.frame(x)) {
+      if (!NROW(x)) x else x[1L,]
+    }
+  } else if ("package:xts" %chin% search()) {
+    if (!requireNamespace("xts", quietly=TRUE))
+      stop("internal error, package:xts is on search path but could not be loaded via requireNamespace") # nocov
+    if (isTRUE(getOption("datatable.verbose", FALSE)))
+      cat("first: using xts::first\n")
+    xts::first(x, n=n, ...)
   } else {
-    # fix with suggestion from Joshua, #1347
-    if (!"package:xts" %chin% search()) {
-      head(x, n=n, ...) # nocov
-    } else xts::first(x, n=n, ...) # nocov
+    head(x, n=n, ...) # nocov
   }
 }

inst/tests/tests.Rraw

@@ -16041,6 +16041,25 @@ test(2107.3, names(DT), c('A','b','c'))
 setnames(DT, -(1:2), toupper)
 test(2107.4, names(DT), c('A','b','C'))
 
+# first and last should no longer load xts namespace, #3857
+x = as.POSIXct("2019-09-09")+0:1
+old = options(datatable.verbose=TRUE)
+test(2108.01, last(x), x[length(x)], notOutput="xts")
+test(2108.02, first(x), x[1L], notOutput="xts")
+if (test_xts) {
+  xt = xts(1:2, x)
+  test(2108.03, last(xt, 2L), xt, output="last: using xts::last")
+  test(2108.04, first(xt, 2L), xt, output="first: using xts::first")
+  xt = xts(matrix(1:4, 2L, 2L), x)
+  test(2108.05, last(xt, 2L), xt, output="last: using xts::last")
+  test(2108.06, first(xt, 2L), xt, output="first: using xts::first")
+}
+# first on empty df now match head(df, n=1L), #3858
+df = data.frame(a=integer(), b=integer())
+test(2108.11, first(df), df, notOutput="xts")
+test(2108.12, tail(df), df, notOutput="xts")
+options(old)
+
 
 ###################################
 #  Add new tests above this line  #