From b36f621362560eb29c4ac3006f56e91025c563ed Mon Sep 17 00:00:00 2001 From: mattdowle Date: Mon, 18 Nov 2019 14:01:12 -0800 Subject: [PATCH 1/2] backport setAttrib feature in R 3.2.0 --- R/data.table.R | 18 +++++++++--------- src/wrappers.c | 11 +++++++++-- 2 files changed, 18 insertions(+), 11 deletions(-) diff --git a/R/data.table.R b/R/data.table.R index 08ef6872b1..f8de1462c0 100644 --- a/R/data.table.R +++ b/R/data.table.R @@ -855,9 +855,11 @@ replace_dot_alias = function(e) { jvnames = NULL drop_dot = function(x) { - tt = x %chin% c(".N",".I",".GRP",".BY") - if (any(tt)) x[tt] = substring(x[tt], 2L) - x + if (length(x)!=1L) stop("Internal error: drop_dot passed ",length(x)," items") # nocov + if (identical(substring(x<-as.character(x),1L,1L), ".") && x %chin% c(".N",".I",".GRP",".BY")) + substring(x, 2L) + else + x } # handle auto-naming of last item of j (e.g. within {} or if/else, #2478) # e.g. DT[, .(a=sum(v), v, .N), by=] should create columns named a, v, N @@ -870,18 +872,16 @@ replace_dot_alias = function(e) { nm = names(q[-1L]) # check list(a=sum(v),v) if (is.null(nm)) nm = rep.int("", qlen-1L) # attempt to auto-name unnamed columns - idx = which(!nzchar(nm)) - for (jj in idx) { + for (jj in which(nm=="")) { thisq = q[[jj + 1L]] if (missing(thisq)) stop("Item ", jj, " of the .() or list() passed to j is missing") #3507 - if (is.name(thisq)) nm[jj] = as.character(thisq) + if (is.name(thisq)) nm[jj] = drop_dot(thisq) # TO DO: if call to a[1] for example, then call it 'a' too } - nm[idx] = drop_dot(nm[idx]) if (!is.null(jvnames) && any(idx <- nm != jvnames)) warning("Different branches of j expression produced different auto-named columns: ", brackify(sprintf('%s!=%s', nm[idx], jvnames[idx])), '; using the most "last" names', call. = FALSE) jvnames <<- nm # TODO: handle if() list(a, b) else list(b, a) better - setattr(q, "names", NULL) # drops the names from the list so it's faster to eval the j for each group. We'll put them back afterwards on the result. + setattr(q, "names", NULL) # drops the names from the list so it's faster to eval the j for each group; reinstated at the end on the result. } return(q) # else empty list is needed for test 468: adding an empty list column } @@ -899,7 +899,7 @@ replace_dot_alias = function(e) { } if (is.name(jsub)) { # j is a single unquoted column name - if (jsub!=".SD") jvnames = drop_dot(as.character(jsub)) + if (jsub!=".SD") jvnames = drop_dot(jsub) # jsub is list()ed after it's eval'd inside dogroups. } else jsub = do_j_names(jsub) # else maybe a call to transform or something which returns a list. av = all.vars(jsub,TRUE) # TRUE fixes bug #1294 which didn't see b in j=fns[[b]](c) diff --git a/src/wrappers.c b/src/wrappers.c index af9ac39ccf..9101987d9a 100644 --- a/src/wrappers.c +++ b/src/wrappers.c @@ -22,11 +22,18 @@ SEXP setattrib(SEXP x, SEXP name, SEXP value) UNPROTECT(1); return(x); } - setAttrib(x, name, - MAYBE_REFERENCED(value) ? duplicate(value) : value); + if (isNull(value) && isPairList(x) && strcmp(CHAR(STRING_ELT(name,0)),"names")==0) { + // backport fix in R 3.2.0 to support R 3.1.0; #4048 #3802 + // apply this backport always (i.e. in R >=3.2.0 too) to avoid a switch on version number or feature test (to avoid more code, tests and nocov) + for (SEXP t=x; t!=R_NilValue; t=CDR(t)) { + SET_TAG(t, R_NilValue); + } + } else { + setAttrib(x, name, MAYBE_REFERENCED(value) ? duplicate(value) : value); // duplicate is temp fix to restore R behaviour prior to R-devel change on 10 Jan 2014 (r64724). // TO DO: revisit. Enough to reproduce is: DT=data.table(a=1:3); DT[2]; DT[,b:=2] // ... Error: selfrefnames is ok but tl names [1] != tl [100] + } return(R_NilValue); } From 87c856c4a834832ac9fc1e3115726aa7d0e6f417 Mon Sep 17 00:00:00 2001 From: mattdowle Date: Mon, 18 Nov 2019 14:05:46 -0800 Subject: [PATCH 2/2] add test that user explicit naming .(.N=.N) does not drop dot --- inst/tests/tests.Rraw | 2 ++ 1 file changed, 2 insertions(+) diff --git a/inst/tests/tests.Rraw b/inst/tests/tests.Rraw index 0c6494d594..64f9199719 100644 --- a/inst/tests/tests.Rraw +++ b/inst/tests/tests.Rraw @@ -16377,6 +16377,8 @@ test(2121.2, DT[ , {{{b = b; .(a, b = b + 1)}}}], DT[ , .(a, b=b+1)]) test(2121.3, DT[ , if (.N > 1L) .(b), by=a], DT[1:2]) test(2121.4, DT[ , if (.N > 1L) .(b) else .(c=b), by=a], DT[ , .(a, c=b)], warning="Different branches of j expression produced different auto-named columns") +test(2121.5, DT[, .(.N=.N), by=a], data.table(a=c(1,2), .N=2:1)) # user supplied names preside over autoname dropping leading dot + ################################### # Add new tests above this line #