From b88112ae1cd309838252d425081422140595631c Mon Sep 17 00:00:00 2001 From: Michael Chirico Date: Wed, 27 Dec 2023 00:33:18 +0800 Subject: [PATCH] Progress some planned deprecations (#5841) * Progress some planned deprecations * remove y= from test() * update tests * vestigial * Fix implicit inter-test dependence --- NEWS.md | 5 +++++ R/fwrite.R | 2 +- R/onLoad.R | 7 +++---- R/setkey.R | 19 ++++--------------- inst/tests/tests.Rraw | 25 +++++++++---------------- man/data.table.Rd | 1 - 6 files changed, 22 insertions(+), 37 deletions(-) diff --git a/NEWS.md b/NEWS.md index 48f7c529e..3eea18c9a 100644 --- a/NEWS.md +++ b/NEWS.md @@ -609,5 +609,10 @@ 15. Thanks to @ssh352, Václav Tlapák, Cole Miller, András Svraka and Toby Dylan Hocking for reporting and bisecting a significant performance regression in dev. This was fixed before release thanks to a PR by Jan Gorecki, [#5463](https://github.com/Rdatatable/data.table/pull/5463). +16. `key(x) <- value` is now fully deprecated (from warning to error). Use `setkey()` to set a table's key. We started warning not to use this approach in 2012, with a stronger warning starting in 2019 (1.12.2). This function will be removed in the next release. + +17. Argument `logicalAsInt` to `fwrite()` now warns. Use `logical01` instead. We stated the intention to begin removing this option in 2018 (v1.11.0). It will be upgraded to an error in the next release before being removed in the subsequent release. + +18. Option `datatable.CJ.names` no longer has any effect, after becoming `TRUE` by default in v1.12.2 (2019). Setting it now gives a warning, which will be dropped in the next release. # data.table v1.14.10 (Dec 2023) back to v1.10.0 (Dec 2016) has been moved to [NEWS.1.md](https://github.com/Rdatatable/data.table/blob/master/NEWS.1.md) diff --git a/R/fwrite.R b/R/fwrite.R index e1484b9e3..20f1c70f5 100644 --- a/R/fwrite.R +++ b/R/fwrite.R @@ -27,7 +27,7 @@ fwrite = function(x, file="", append=FALSE, quote="auto", if (!missing(logical01) && !missing(logicalAsInt)) stopf("logicalAsInt has been renamed logical01. Use logical01 only, not both.") if (!missing(logicalAsInt)) { - # TODO: warningf("logicalAsInt has been renamed logical01 for consistency with fread. It will work fine but please change to logical01 at your convenience so we can remove logicalAsInt in future.") + warningf("logicalAsInt has been renamed logical01 for consistency with fread. It works fine for now but please change to logical01 at your convenience so we can remove logicalAsInt in future.") logical01 = logicalAsInt logicalAsInt=NULL } diff --git a/R/onLoad.R b/R/onLoad.R index b4ebeafdf..9080e328f 100644 --- a/R/onLoad.R +++ b/R/onLoad.R @@ -90,10 +90,9 @@ eval(parse(text=paste0("options(",i,"=",opts[i],")"))) } - if (!is.null(getOption("datatable.old.bywithoutby"))) - warningf("Option 'datatable.old.bywithoutby' has been removed as warned for 2 years. It is now ignored. Please use by=.EACHI instead and stop using this option.") - if (!is.null(getOption("datatable.old.unique.by.key"))) - warningf("Option 'datatable.old.unique.by.key' has been removed as warned for 4 years. It is now ignored. Please use by=key(DT) instead and stop using this option.") + # default TRUE from v1.12.0, FALSE before. Now ineffectual. Remove this warning after 1.15.0. + if (!is.null(getOption("datatable.CJ.names"))) + warningf("Option 'datatable.CJ.names' no longer has any effect, as promised for 4 years. It is now ignored. Manually name `...` entries as needed if you still prefer the old behavior.") # Test R behaviour that changed in v3.1 and is now depended on x = 1L:3L diff --git a/R/setkey.R b/R/setkey.R index 5f3027a2d..84488a803 100644 --- a/R/setkey.R +++ b/R/setkey.R @@ -18,15 +18,9 @@ setindexv = function(x, cols, verbose=getOption("datatable.verbose")) { } } -# upgrade to error after Mar 2020. Has already been warning since 2012, and stronger warning in Mar 2019 (note in news for 1.12.2); #3399 +# Has been warning since 2012, with stronger warning in Mar 2019 (note in news for 1.12.2); #3399 "key<-" = function(x,value) { - warningf("key(x)<-value is deprecated and not supported. Please change to use setkey() with perhaps copy(). Has been warning since 2012 and will be an error in future.") - setkeyv(x,value) - # The returned value here from key<- is then copied by R before assigning to x, it seems. That's - # why we can't do anything about it without a change in R itself. If we return NULL (or invisible()) from this key<- - # method, the table gets set to NULL. So, although we call setkeyv(x,cols) here, and that doesn't copy, the - # returned value (x) then gets copied by R. - # So, solution is that caller has to call setkey or setkeyv directly themselves, to avoid <- dispatch and its copy. + stopf("key(x)<-value is deprecated and not supported. Please change to use setkey() with perhaps copy(). Has been warning since 2012.") } setkeyv = function(x, cols, verbose=getOption("datatable.verbose"), physical=TRUE) @@ -325,13 +319,8 @@ CJ = function(..., sorted = TRUE, unique = FALSE) # Cross Join will then produce a join table with the combination of all values (cross product). # The last vector is varied the quickest in the table, so dates should be last for roll for example l = list(...) - if (isFALSE(getOption("datatable.CJ.names", TRUE))) { # default TRUE from v1.12.0, FALSE before. TODO: remove option in v1.13.0 as stated in news - if (is.null(vnames <- names(l))) vnames = paste0("V", seq_len(length(l))) - else if (any(tt <- vnames=="")) vnames[tt] = paste0("V", which(tt)) - } else { - vnames = name_dots(...)$vnames - if (any(tt <- vnames=="")) vnames[tt] = paste0("V", which(tt)) - } + vnames = name_dots(...)$vnames + if (any(tt <- !nzchar(vnames))) vnames[tt] = paste0("V", which(tt)) dups = FALSE # fix for #1513 for (i in seq_along(l)) { y = l[[i]] diff --git a/inst/tests/tests.Rraw b/inst/tests/tests.Rraw index 35158857d..a9000e492 100644 --- a/inst/tests/tests.Rraw +++ b/inst/tests/tests.Rraw @@ -1591,11 +1591,9 @@ test(505, DT[J(a=1,b=6),sum(i.b*b),by=.EACHI]$V1, 24) # 24 now 'double' because # Test := after a key<- DT = data.table(a=3:1,b=4:6) -test(506, key(DT)<-"a", "a", warning="deprecated") -test(508, DT, data.table(a=1:3,b=6:4,key="a")) -test(509, DT[,b:=10L], data.table(a=1:3,b=10L,key="a")) -test(510, DT[,c:=11L], data.table(a=1:3,b=10L,c=11L,key="a"), # no warning between 1.8.3 and 1.12.2 due to (now removed) setmutable and SET_NAMED in setalloccol, #3729 - warning="Invalid .internal.selfref detected and fixed") # but the warning makes sense after the (deprecated) key(DT)<- above, so this warns again from 1.12.4 +test(506, key(DT)<-"a", error="deprecated") + +# tests 508, 509, 510 related to follow-up operations after key<-, which are now irrelevant # Test new functons chmatch and %chin% y=letters @@ -2950,12 +2948,8 @@ test(995, DT[CJ(c(5,3), c(5,1), sorted=FALSE)], OUT) xx <- factor(letters[1:2], ordered=TRUE) yy <- sample(2L) yy_sort = base::sort.int(yy) -old = options(datatable.CJ.names=FALSE) -test(996.01, CJ(xx, yy), setkey(data.table(rep(xx, each=2L), rep(yy_sort, 2L)))) -test(996.02, CJ(a = xx, yy), setkey(data.table(a = rep(xx, each=2L), V2 = rep(yy_sort, 2L)))) -options(datatable.CJ.names=TRUE) +# 996.01, 996.02 tested the now-ineffectual datatable.CJ.names option test(996.03, CJ(xx, yy), setkey(data.table(xx = rep(xx, each=2L), yy = rep(yy_sort, 2L)))) -options(old) # #3597 -- CJ properly informs sorted can't apply to list input test(996.04, CJ(list(1:2, 3L)), error = "non-atomic, which can't be sorted") @@ -7305,12 +7299,10 @@ test(1524, ans1, ans2) x = c(1, 2, 1) y = c(5, 8, 8, 4) w = c(10, 12, 12, 13) # already sorted but has dups; more efficient case to cover -options(datatable.CJ.names=FALSE) -test(1525.1, CJ(x, y, unique=TRUE), CJ(V1=c(1,2), V2=c(4,5,8))) -test(1525.2, CJ(x, z=y, unique=TRUE), ans<-data.table(V1=rep(c(1,2), each=3), z=c(4,5,8), key="V1,z")) # naming of one but not both, too -options(datatable.CJ.names=TRUE) +# tests 1525.1, 1525.2 tested the now-ineffectual datatable.CJ.names option. +ans<-data.table(V1=rep(c(1,2), each=3), z=c(4,5,8), key="V1,z") test(1525.3, CJ(x, y, unique=TRUE), CJ( x=c(1,2), y=c(4,5,8))) -test(1525.4, CJ(x, z=y, unique=TRUE), setnames(ans,c("x","z"))) +test(1525.4, CJ(x, z=y, unique=TRUE), setnames(copy(ans),c("x","z"))) test(1525.5, CJ(x, w, unique=TRUE), data.table(x=(rep(c(1,2), each=3)), w=c(10,12,13), key="x,w")) # `key` argument fix for `setDT` when input is already a `data.table`, #1169 @@ -10779,7 +10771,8 @@ test(1736.05, capture.output(fwrite(DT, sep='|', sep2=c("c(",",",")"), logical01 "2|c(15,16,17,18)|c(1.2,2.3,3.4,3.14159265358979,-9)", "3|c(7)|c(foo,bar)", "4|c(9,10)|c(TRUE,TRUE,FALSE)")) test(1736.06, capture.output(fwrite(DT, sep='|', sep2=c("{",",","}"), logicalAsInt=TRUE)), c("A|B|C", "1|{1,2,3,4,5,6,7,8,9,10}|{s,t,u,v,w}", - "2|{15,16,17,18}|{1.2,2.3,3.4,3.14159265358979,-9}", "3|{7}|{foo,bar}", "4|{9,10}|{1,1,0}")) + "2|{15,16,17,18}|{1.2,2.3,3.4,3.14159265358979,-9}", "3|{7}|{foo,bar}", "4|{9,10}|{1,1,0}"), + warning="logicalAsInt has been renamed logical01") DT = data.table(A=c("foo","ba|r","baz")) test(1736.07, capture.output(fwrite(DT,na="")), c("A","foo","ba|r","baz")) # no list column so no need to quote test(1736.08, capture.output(fwrite(DT)), c("A","foo","ba|r","baz")) diff --git a/man/data.table.Rd b/man/data.table.Rd index b8011b422..2e326fed0 100644 --- a/man/data.table.Rd +++ b/man/data.table.Rd @@ -344,7 +344,6 @@ setkey(kDT,x) # set a 1-column key. No quotes, for conve setkeyv(kDT,"x") # same (v in setkeyv stands for vector) v="x" setkeyv(kDT,v) # same -# key(kDT)<-"x" # copies whole table, please use set* functions instead haskey(kDT) # TRUE key(kDT) # "x"