From 12ecd27c945c94a3b99a1e7dbbd7d06f45edef32 Mon Sep 17 00:00:00 2001 From: mattdowle Date: Wed, 16 Oct 2019 19:36:41 -0700 Subject: [PATCH] check data.table installed with the same major version of R --- .dev/CRAN_Release.cmd | 1 + .dev/cc.R | 1 + NAMESPACE | 13 ++++++++++++- R/data.table.R | 11 ++++------- R/onLoad.R | 21 +++++++++++++++------ 5 files changed, 33 insertions(+), 14 deletions(-) diff --git a/.dev/CRAN_Release.cmd b/.dev/CRAN_Release.cmd index 92b810b07d..6c09f88d2a 100644 --- a/.dev/CRAN_Release.cmd +++ b/.dev/CRAN_Release.cmd @@ -207,6 +207,7 @@ tar xvf R-devel.tar.gz mv R-devel R-devel-strict-gcc tar xvf R-devel.tar.gz mv R-devel R-devel-strict-clang +tar xvf R-devel.tar.gz # use gcc-8 and clang-8 in CC=, or latest available in `apt cache search gcc-` or `clang-` diff --git a/.dev/cc.R b/.dev/cc.R index 2441de5e50..329935c827 100644 --- a/.dev/cc.R +++ b/.dev/cc.R @@ -79,6 +79,7 @@ cc = function(test=TRUE, clean=FALSE, debug=FALSE, omp=!debug, cc_dir, path=Sys. for (i in seq_along(xx$.External)) assign(xx$.External[[i]]$name, xx$.External[[i]]$address, envir=.GlobalEnv) sourceDir(paste0(path,"/R")) + if (base::getRversion()<"4.0.0") rm(list=c("rbind.data.table","cbind.data.table"), envir=.GlobalEnv) # 3968 follow up assign("testDir", function(x)paste0(path,"/inst/tests/",x), envir=.GlobalEnv) .onLoad() if (is.logical(test) && isTRUE(test)) test.data.table() else if (is.character(test)) test.data.table(script=test) diff --git a/NAMESPACE b/NAMESPACE index 193b4dedae..49bd3a35d6 100644 --- a/NAMESPACE +++ b/NAMESPACE @@ -84,10 +84,21 @@ S3method(as.matrix, data.table) if (getRversion() >= "4.0.0") { # this version number must be the same as in .onLoad # fix in R in Sep 2019 (#3948) makes c|rbind S3 dispatch work; see FAQ 2.24. - # if we register these methods always though, the previous workaround no longer works in R<4.0.0. Hence only register in R>=4.0.0. + # if we register these (new in v1.12.6) methods always though, the previous workaround no longer works in R<4.0.0. Hence only register in R>=4.0.0. S3method(cbind, data.table) S3method(rbind, data.table) } +# else { +# # and if we export but don't register in R < 4.0.0 we get this note: +# # > Found the following apparent S3 methods exported but not registered: +# # > cbind.data.table rbind.data.table +# # in addition to errors in tests 324, 326, 414.1, 414.2, 442, 445, 451 +# export(cbind.data.table) +# export(rbind.data.table) +# # A revdep using rbind.data.frame() directly before (which data.table changed in base) should change to rbind() generic and that should work +# # in all combinations of R before/after 4.0.0 and data.table before/after 1.12.6, so long as data.table is installed using the same major +# # version of R (and that is checked in .onLoad with error if not). +# } export(.rbind.data.table) # continue to export for now because it has been exported in the past so it may be depended on S3method(dim, data.table) S3method(dimnames, data.table) diff --git a/R/data.table.R b/R/data.table.R index 920b0dbc69..e774a7f4a7 100644 --- a/R/data.table.R +++ b/R/data.table.R @@ -2531,16 +2531,13 @@ chgroup = function(x) { if (length(o)) as.vector(o) else seq_along(x) # as.vector removes the attributes } -.rbind.data.table = function(..., use.names=TRUE, fill=FALSE, idcol=NULL) { +# plain rbind and cbind methods are registered using S3method() in NAMESPACE only from R>=4.0.0; #3948 +rbind.data.table = function(..., use.names=TRUE, fill=FALSE, idcol=NULL) { l = lapply(list(...), function(x) if (is.list(x)) x else as.data.table(x)) #1626; e.g. psych binds a data.frame|table with a matrix rbindlist(l, use.names, fill, idcol) } - -if (base::getRversion() >= "4.0.0") { #3948 - # these cause cc() to fail if present in .GlobalEnv in dev with R<4.0.0 - cbind.data.table = data.table - rbind.data.table = .rbind.data.table -} +cbind.data.table = data.table +.rbind.data.table = rbind.data.table # the workaround using this in FAQ 2.24 is still applied to support R < 4.0.0 rbindlist = function(l, use.names="check", fill=FALSE, idcol=NULL) { if (is.null(l)) return(null.data.table()) diff --git a/R/onLoad.R b/R/onLoad.R index ccbd307926..931bbcdb47 100644 --- a/R/onLoad.R +++ b/R/onLoad.R @@ -17,12 +17,21 @@ .onLoad = function(libname, pkgname) { # Runs when loaded but not attached to search() path; e.g., when a package just Imports (not Depends on) data.table - if (!exists("test.data.table", .GlobalEnv, inherits=FALSE) && # check when installed package is loaded but skip when developing the package with cc() - (dllV<-if(is.loaded("CdllVersion",PACKAGE="datatable")).Call(CdllVersion)else"before 1.12.0") != (RV<-packageVersion("data.table"))) { - # ^^ not dot as this is the name of the dll file, #3282 - dll = if (.Platform$OS.type=="windows") "dll" else "so" - # https://bugs.r-project.org/bugzilla/show_bug.cgi?id=17478 - stop("The datatable.",dll," version (",dllV,") does not match the package (",RV,"). Please close all R sessions to release the old ",toupper(dll)," and reinstall data.table in a fresh R session. The root cause is that R's package installer can in some unconfirmed circumstances leave a package in a state that is apparently functional but where new R code is calling old C code silently: https://bugs.r-project.org/bugzilla/show_bug.cgi?id=17478. Once a package is in this mismatch state it may produce wrong results silently until you next upgrade the package. Please help by adding precise circumstances to 17478 to move the status to confirmed. This mismatch between R and C code can happen with any package not just data.table. It is just that data.table has added this check.") + if (!exists("test.data.table", .GlobalEnv, inherits=FALSE)) { + # check when installed package is loaded but skip when developing the package with cc() + dllV = if (is.loaded("CdllVersion",PACKAGE="datatable")) .Call(CdllVersion) else "before 1.12.0" + # ^^ no dot as this is the name of the dll file, #3282 + RV = packageVersion("data.table") + if (dllV != RV) { + dll = if (.Platform$OS.type=="windows") "dll" else "so" + # https://bugs.r-project.org/bugzilla/show_bug.cgi?id=17478 + stop("The datatable.",dll," version (",dllV,") does not match the package (",RV,"). Please close all R sessions to release the old ",toupper(dll)," and reinstall data.table in a fresh R session. The root cause is that R's package installer can in some unconfirmed circumstances leave a package in a state that is apparently functional but where new R code is calling old C code silently: https://bugs.r-project.org/bugzilla/show_bug.cgi?id=17478. Once a package is in this mismatch state it may produce wrong results silently until you next upgrade the package. Please help by adding precise circumstances to 17478 to move the status to confirmed. This mismatch between R and C code can happen with any package not just data.table. It is just that data.table has added this check.") + } + builtUsing = readRDS(system.file("Meta/package.rds",package="data.table"))$Built$R + if (!identical(base::getRversion()>="4.0.0", builtUsing>="4.0.0")) { + stop("This is R ", base::getRversion(), " but data.table has been installed using R ",builtUsing,". The major version must match. Please reinstall data.table.") + # the if(R>=4.0.0) in NAMESPACE when registering S3 methods rbind.data.table and cbind.data.table happens on install; #3968 + } } # c|rbind S3 dispatch now works in R-devel from Sep 2019; #3948 and FAQ 2.24, and R-devel is 4.0.0 as of now. I wanted to test