diff --git a/NEWS.md b/NEWS.md index 73e8fd5608..ef19ccd191 100644 --- a/NEWS.md +++ b/NEWS.md @@ -167,6 +167,8 @@ 31. `fwrite(DF, row.names=TRUE)` where `DF` has specific integer rownames (e.g. using `rownames(DF) <- c(10L,20L,30L)`) would ignore the integer rownames and write the row numbers instead, [#4957](https://github.com/Rdatatable/data.table/issues/4957). Thanks to @dgarrimar for reporting and @ColeMiller1 for the PR. Further, when `quote='auto'` (default) and the rownames are integers (either default or specific), they are no longer quoted. +32. `test.data.table()` would fail on test 1894 if the variable `z` was defined by the user, [#3705](https://github.com/Rdatatable/data.table/issues/3705). The test suite already ran in its own separate environment. That environment's parent is no longer `.GlobalEnv` to isolate it further. Thanks to Michael Chirico for reporting, and Matt Dowle for the PR. + ## NOTES 1. New feature 29 in v1.12.4 (Oct 2019) introduced zero-copy coercion. Our thinking is that requiring you to get the type right in the case of `0` (type double) vs `0L` (type integer) is too inconvenient for you the user. So such coercions happen in `data.table` automatically without warning. Thanks to zero-copy coercion there is no speed penalty, even when calling `set()` many times in a loop, so there's no speed penalty to warn you about either. However, we believe that assigning a character value such as `"2"` into an integer column is more likely to be a user mistake that you would like to be warned about. The type difference (character vs integer) may be the only clue that you have selected the wrong column, or typed the wrong variable to be assigned to that column. For this reason we view character to numeric-like coercion differently and will warn about it. If it is correct, then the warning is intended to nudge you to wrap the RHS with `as.()` so that it is clear to readers of your code that a coercion from character to that type is intended. For example : diff --git a/R/test.data.table.R b/R/test.data.table.R index a8a19522f4..0c7fbeb23a 100644 --- a/R/test.data.table.R +++ b/R/test.data.table.R @@ -1,16 +1,18 @@ test.data.table = function(script="tests.Rraw", verbose=FALSE, pkg=".", silent=FALSE, showProgress=interactive()&&!silent) { stopifnot(isTRUEorFALSE(verbose), isTRUEorFALSE(silent), isTRUEorFALSE(showProgress)) - if (exists("test.data.table", .GlobalEnv,inherits=FALSE)) { + if (exists("test.data.table", .GlobalEnv, inherits=FALSE)) { # package developer # nocov start if ("package:data.table" %chin% search()) stopf("data.table package is loaded. Unload or start a fresh R session.") rootdir = if (pkg!="." && pkg %chin% dir()) file.path(getwd(), pkg) else Sys.getenv("PROJ_PATH") subdir = file.path("inst","tests") + env = new.env(parent=.GlobalEnv) # in dev cc() sources all functions in .GlobalEnv # nocov end } else { # i) R CMD check and ii) user running test.data.table() rootdir = getNamespaceInfo("data.table","path") subdir = "tests" + env = new.env(parent=parent.env(.GlobalEnv)) # when user runs test.data.table() we don't want their variables in .GlobalEnv affecting tests, #3705 } fulldir = file.path(rootdir, subdir) @@ -93,7 +95,6 @@ test.data.table = function(script="tests.Rraw", verbose=FALSE, pkg=".", silent=F cat("getDTthreads(verbose=TRUE):\n") # for tracing on CRAN; output to log before anything is attempted getDTthreads(verbose=TRUE) # includes the returned value in the verbose output (rather than dangling '[1] 4'); e.g. "data.table is using 4 threads" catf("test.data.table() running: %s\n", fn) # print fn to log before attempting anything on it (in case it is missing); on same line for slightly easier grep - env = new.env(parent=.GlobalEnv) assign("testDir", function(x) file.path(fulldir, x), envir=env) # are R's messages being translated to a foreign language? #3039, #630 diff --git a/inst/tests/nafill.Rraw b/inst/tests/nafill.Rraw index dcaa0f40d4..1e5107fb71 100644 --- a/inst/tests/nafill.Rraw +++ b/inst/tests/nafill.Rraw @@ -15,8 +15,10 @@ sugg = c( "nanotime" ) for (s in sugg) { - assign(paste0("test_",s), loaded<-suppressWarnings(suppressMessages(require(s, character.only=TRUE)))) - if (!loaded) cat("\n**** Suggested package",s,"is not installed. Tests using it will be skipped.\n\n") + assign(paste0("test_",s), loaded<-suppressWarnings(suppressMessages( + library(s, character.only=TRUE, logical.return=TRUE, quietly=TRUE, warn.conflicts=FALSE, pos="package:base") # attach at the end for #5101 + ))) + if (!loaded) cat("\n**** Suggested package",s,"is not installed or has dependencies missing. Tests using it will be skipped.\n\n") } x = 1:10 diff --git a/inst/tests/other.Rraw b/inst/tests/other.Rraw index 89ce49b00f..a346a9e56b 100644 --- a/inst/tests/other.Rraw +++ b/inst/tests/other.Rraw @@ -15,7 +15,9 @@ INT = data.table:::INT if (any(duplicated(pkgs))) stop("Packages defined to be loaded for integration tests in 'inst/tests/other.Rraw' contains duplicates.") -f = function(pkg) suppressMessages(isTRUE(require(pkg, character.only=TRUE, quietly=TRUE, warn.conflicts=FALSE))) +f = function(pkg) suppressWarnings(suppressMessages(isTRUE( + library(pkg, character.only=TRUE, logical.return=TRUE, quietly=TRUE, warn.conflicts=FALSE, pos="package:base") # attach at the end for #5101 +))) loaded = sapply(pkgs, f) if (any(!loaded)) { stop("test.data.table('other.Rraw') is missing required package(s): ", paste(names(loaded)[!loaded], collapse=", "), ". If you can't install them and this is R CMD check, please set environment variable TEST_DATA_TABLE_WITH_OTHER_PACKAGES back to the default, false.") diff --git a/inst/tests/tests.Rraw b/inst/tests/tests.Rraw index fb3cb363da..0578b5b30f 100644 --- a/inst/tests/tests.Rraw +++ b/inst/tests/tests.Rraw @@ -96,8 +96,10 @@ sugg = c( # zoo # In DESCRIPTION:Suggests otherwise R CMD check warning: '::' or ':::' import not declared from: 'zoo'; it is tested in other.Rraw though ) for (s in sugg) { - assign(paste0("test_",s), loaded<-suppressWarnings(suppressMessages(require(s, character.only=TRUE)))) - if (!loaded) cat("\n**** Suggested package",s,"is not installed. Tests using it will be skipped.\n\n") + assign(paste0("test_",s), loaded<-suppressWarnings(suppressMessages( + library(s, character.only=TRUE, logical.return=TRUE, quietly=TRUE, warn.conflicts=FALSE, pos="package:base") # attach at the end for #5101 + ))) + if (!loaded) cat("\n**** Suggested package",s,"is not installed or has dependencies missing. Tests using it will be skipped.\n\n") } test_longdouble = isTRUE(capabilities()["long.double"]) && identical(as.integer(.Machine$longdouble.digits), 64L)