Merge branch 'master' into issue1945

Author: venom1204

.ci/README.md

@@ -12,11 +12,15 @@ Test jobs:
 - `test-lin-rel-cran` - `--as-cran` on Linux, strict test for final status of `R CMD check`.
 - `test-lin-dev-gcc-strict-cran` - `--as-cran` on Linux, `r-devel` built with `-enable-strict-barrier --disable-long-double`, test for compilation warnings, test for new NOTEs/WARNINGs from `R CMD check`.
 - `test-lin-dev-clang-cran` - same as `gcc-strict` job but R built with `clang` and  no `--enable-strict-barrier --disable-long-double` flags.
-- `test-lin-310-cran` - R 3.1.0 on Linux, stated R dependency version.
+- `test-lin-ancient-cran` - Stated R dependency version (currently 3.3.0) on Linux.
+- `test-lin-dev-san` - `r-devel` on Linux built with `clang -fsanitize=address,undefined` (including LeakSanitizer), test for sanitizer output in tests and examples.
 - `test-win-rel` - `r-release` on Windows.
 - `test-win-dev` - `r-devel` on Windows.
 - `test-win-old` - `r-oldrel` on Windows.
-- `test-mac-rel` - macOS build not yet available, see [#3326](https://github.com/Rdatatable/data.table/issues/3326) for status
+- `test-mac-rel` - `r-release` on macOS.
+- `test-mac-old` - `r-oldrel` on macOS.
+
+The CI steps for the tests are [required](https://github.com/Rdatatable/data.table/blob/55eb0f160b169398d51f138131c14a66c86e5dc9/.ci/publish.R#L162-L168) to be named according to the pattern `test-(lin|win|mac)-<R version>[-<suffix>]*`, where `<R version>` is `rel`, `dev`, `old`, `ancient`, or three digits comprising an R version (e.g. `362` corresponding to R-3.6.2).
 
 Tests jobs are allowed to fail, summary and logs of test jobs are later published at _CRAN-like checks_ page, see artifacts below.
 

NEWS.md

@@ -14,6 +14,8 @@
 
 3. `fread(keepLeadingZeros=TRUE)` now correctly parses dates with components with leading zeros as dates instead of strings, [#6851](https://github.com/Rdatatable/data.table/issues/6851). Thanks @TurnaevEvgeny for the report and @ben-schwen for the fix.
 
+4. `as.data.table()` now properly handles keys: specifying keys sets them, omitting keys preserves existing ones, and setting `key=NULL` clears them, [#6859](https://github.com/Rdatatable/data.table/issues/6859). Thanks @brookslogan for the report and @Mukulyadav2004 for the fix.
+
 ## NOTES
 
 1. Continued work to remove non-API C functions, [#6180](https://github.com/Rdatatable/data.table/issues/6180). Thanks Ivan Krylov for the PRs and for writing a clear and concise guide about the R API: https://aitap.codeberg.page/R-api/.

R/as.data.table.R

@@ -214,8 +214,8 @@ as.data.table.list = function(x,
 }
 
 as.data.table.data.frame = function(x, keep.rownames=FALSE, key=NULL, ...) {
-  if (is.data.table(x)) return(as.data.table.data.table(x)) # S3 is weird, #6739. Also # nocov; this is tested in 2302.{2,3}, not sure why it doesn't show up in coverage.
-  if (!identical(class(x), "data.frame")) return(as.data.table(as.data.frame(x)))
+  if (is.data.table(x)) return(as.data.table.data.table(x, key=key)) # S3 is weird, #6739. Also # nocov; this is tested in 2302.{2,3}, not sure why it doesn't show up in coverage.
+  if (!identical(class(x), "data.frame")) return(as.data.table(as.data.frame(x), keep.rownames=keep.rownames, key=key, ...))
   if (!isFALSE(keep.rownames)) {
     # can specify col name to keep.rownames, #575; if it's the same as key,
     #   kludge it to 'rn' since we only apply the new name afterwards, #4468
@@ -228,7 +228,7 @@ as.data.table.data.frame = function(x, keep.rownames=FALSE, key=NULL, ...) {
   if (any(cols_with_dims(x))) {
     # a data.frame with a column that is data.frame needs to be expanded; test 2013.4
     # x may be a class with [[ method that behaves differently, so as.list first for default [[, #4526
-    return(as.data.table.list(as.list(x), keep.rownames=keep.rownames, ...))
+    return(as.data.table.list(as.list(x), keep.rownames=keep.rownames, key = key,...))
   }
   ans = copy(x)  # TO DO: change this deep copy to be shallow.
   setattr(ans, "row.names", .set_row_names(nrow(x)))
@@ -245,13 +245,14 @@ as.data.table.data.frame = function(x, keep.rownames=FALSE, key=NULL, ...) {
   ans
 }
 
-as.data.table.data.table = function(x, ...) {
+as.data.table.data.table = function(x, ..., key=NULL) {
   # as.data.table always returns a copy, automatically takes care of #473
   if (any(cols_with_dims(x))) { # for test 2089.2
-    return(as.data.table.list(x, ...))
+    return(as.data.table.list(x, key = key, ...))
   }
   x = copy(x) # #1681
   # fix for #1078 and #1128, see .resetclass() for explanation.
   setattr(x, 'class', .resetclass(x, "data.table"))
+  if (!missing(key)) setkeyv(x, key)
   x
 }

R/data.table.R

@@ -448,7 +448,7 @@ replace_dot_alias = function(e) {
     if (is.data.frame(i)) {
       if (missing(on)) {
         if (!haskey(x)) {
-          stopf("When i is a data.table (or character vector), the columns to join by must be specified using 'on=' argument (see ?data.table), by keying x (i.e. sorted, and, marked as sorted, see ?setkey), or by sharing column names between x and i (i.e., a natural join). Keyed joins might have further speed benefits on very large data due to x being sorted in RAM.")
+          stopf("When i is a data.table (or character vector), the columns to join by must be specified using the 'on=' argument (see ?data.table); by keying x (i.e., x is sorted and marked as such, see ?setkey); or by using 'on = .NATURAL' to indicate using the shared column names between x and i (i.e., a natural join). Keyed joins might have further speed benefits on very large data due to x being sorted in RAM.")
         }
       } else if (identical(substitute(on), as.name(".NATURAL"))) {
         naturaljoin = TRUE

configure

@@ -122,6 +122,7 @@ detect_openmp () {
     if CPPFLAGS="${CPPFLAGS} -fopenmp" "${R_HOME}/bin/R" CMD SHLIB test-omp.c >> config.log 2>&1; then
       echo "yes"
       export PKG_CFLAGS="${PKG_CFLAGS} -fopenmp"
+      export PKG_LIBS="${PKG_LIBS} -fopenmp"
       export R_OPENMP_ENABLED=1
       return
     else

inst/tests/tests.Rraw

@@ -21084,3 +21084,23 @@ test(2307, { capture.output(print(DT, class = TRUE, show.indices = TRUE)); TRUE
 dt = data.table(date=as.IDate(c(NA, "2014-12-05")))
 test(2308.01, fread("date\nNA\n2014-12-05", keepLeadingZeros=TRUE), dt)
 test(2308.02, fread("date\nNA\n2014-12-05", keepLeadingZeros=FALSE), dt)
+
+# Test that as.data.table.data.table preserves key when explicitly specified but not when omitted
+DF = data.frame(t = c(3:1, 4:5), y = 1:5)
+# data.frame to data.table with key
+test(2309.01, key(as.data.table(DF, key="t")), "t")
+# tibble to data.table with key
+class(DF) = c("tbl_df", "tbl", "data.frame")
+test(2309.02, key(as.data.table(DF, key="t")), "t")
+
+# data.table keyed with "b"
+DT = data.table(a = 1:5, b = 1:5, x = 1:5, key = "b") 
+test(2309.03, key(as.data.table(DT, key="a")), "a")   
+test(2309.04, key(as.data.table(DT)), "b")            
+test(2309.05, key(as.data.table(DT, key=NULL)), NULL) 
+
+# non-keyed data.table
+DT = data.table(a = 1:5, b = 1:5, x = 1:5)
+test(2309.06, key(as.data.table(DT, key="a")), "a")   
+test(2309.07, key(as.data.table(DT)), NULL)          
+test(2309.08, key(as.data.table(DT, key=NULL)), NULL) 

man/as.data.table.Rd

@@ -21,7 +21,7 @@ Functions to check if an object is \code{data.table}, or coerce it if possible.
 \usage{
 as.data.table(x, keep.rownames=FALSE, \dots)
 
-\method{as.data.table}{data.table}(x, \dots)
+\method{as.data.table}{data.table}(x, \dots, key=NULL)
 
 \method{as.data.table}{array}(x, keep.rownames=FALSE, key=NULL, sorted=TRUE,
               value.name="value", na.rm=TRUE, \dots)