From c2f1d2bf5b2b7c5cc80c941960df09ebd6f8fb52 Mon Sep 17 00:00:00 2001 From: Michael Chirico Date: Wed, 27 Mar 2024 19:24:16 -0700 Subject: [PATCH 01/12] Don't cast jsub[[1]] to character if invalid --- R/data.table.R | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/R/data.table.R b/R/data.table.R index 24eff62d5..740ede7d4 100644 --- a/R/data.table.R +++ b/R/data.table.R @@ -253,7 +253,7 @@ replace_dot_alias = function(e) { } if (!missing(j)) { jsub = replace_dot_alias(jsub) - root = if (is.call(jsub)) as.character(jsub[[1L]])[1L] else "" + root = root_name(jsub) if (root == ":" || (root %chin% c("-","!") && jsub[[2L]] %iscall% '(' && jsub[[2L]][[2L]] %iscall% ':') || ( (!length(av<-all.vars(jsub)) || all(startsWith(av, ".."))) && @@ -296,7 +296,7 @@ replace_dot_alias = function(e) { if (root=="{") { if (length(jsub) == 2L) { jsub = jsub[[2L]] # to allow {} wrapping of := e.g. [,{`:=`(...)},] [#376] - root = if (is.call(jsub)) as.character(jsub[[1L]])[1L] else "" + root = root_name(jsub) } else if (length(jsub) > 2L && jsub[[2L]] %iscall% ":=") { #2142 -- j can be {} and have length 1 stopf("You have wrapped := with {} which is ok but then := must be the only thing inside {}. You have something else inside {} as well. Consider placing the {} on the RHS of := instead; e.g. DT[,someCol:={tmpVar1<-...;tmpVar2<-...;tmpVar1*tmpVar2}") @@ -309,10 +309,8 @@ replace_dot_alias = function(e) { jsub = eval(jsub[[2L]], parent.frame(), parent.frame()) # this evals the symbol to return the dynamic expression if (is.expression(jsub)) jsub = jsub[[1L]] # if expression, convert it to call # Note that the dynamic expression could now be := (new in v1.9.7) - root = if (is.call(jsub)) { - jsub = replace_dot_alias(jsub) - as.character(jsub[[1L]])[1L] - } else "" + jsub = replace_dot_alias(jsub) + root = root_name(jsub) } if (root == ":=" || root == "let") { # let(...) as alias for :=(...) (#3795) if (root == "let") @@ -1967,6 +1965,9 @@ replace_dot_alias = function(e) { setalloccol(ans) # TODO: overallocate in dogroups in the first place and remove this line } +# What's the name of the top-level call in 'j'? is.name() check is required for cases like #6026 with complex calls that can't be cast to character +root_name = function(jsub) if (is.call(jsub) && is.name(j1 <- jsub[[1L]])) as.character(j1) else "" + DT = function(x, ...) { #4872 old = getOption("datatable.optimize") if (!is.data.table(x) && old>2L) { From d9fd0703bf969a3f843a0759d87461d2ff7a001a Mon Sep 17 00:00:00 2001 From: Michael Chirico Date: Thu, 28 Mar 2024 08:08:50 -0700 Subject: [PATCH 02/12] Fix for case of lambda in j --- R/data.table.R | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/R/data.table.R b/R/data.table.R index 740ede7d4..81f0dee3c 100644 --- a/R/data.table.R +++ b/R/data.table.R @@ -1966,7 +1966,12 @@ replace_dot_alias = function(e) { } # What's the name of the top-level call in 'j'? is.name() check is required for cases like #6026 with complex calls that can't be cast to character -root_name = function(jsub) if (is.call(jsub) && is.name(j1 <- jsub[[1L]])) as.character(j1) else "" +root_name = function(jsub) { + if (!is.call(jsub)) return("") + if (is.name(j1 <- jsub[[1L]])) return(as.character(j1)) + if (j1 %iscall% "(") return("(") + "" +} DT = function(x, ...) { #4872 old = getOption("datatable.optimize") From 1add608b25401cf8e84d80911cc64c82cece2ff8 Mon Sep 17 00:00:00 2001 From: Michael Chirico Date: Thu, 28 Mar 2024 08:48:36 -0700 Subject: [PATCH 03/12] comment on why '(' handling is needed --- R/data.table.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R/data.table.R b/R/data.table.R index 81f0dee3c..e1db835da 100644 --- a/R/data.table.R +++ b/R/data.table.R @@ -1969,7 +1969,7 @@ replace_dot_alias = function(e) { root_name = function(jsub) { if (!is.call(jsub)) return("") if (is.name(j1 <- jsub[[1L]])) return(as.character(j1)) - if (j1 %iscall% "(") return("(") + if (j1 %iscall% "(") return("(") # needed to prevent with=FALSE from triggering for test 1128 "" } From 857b000137e1ab4c9ec14db7bcf4e64b61f5dba7 Mon Sep 17 00:00:00 2001 From: Michael Chirico Date: Thu, 28 Mar 2024 09:32:08 -0700 Subject: [PATCH 04/12] switch to format() in progress --- R/data.table.R | 36 ++++++++++++++++-------------------- R/utils.R | 6 +++++- inst/tests/tests.Rraw | 5 +++++ 3 files changed, 26 insertions(+), 21 deletions(-) diff --git a/R/data.table.R b/R/data.table.R index e1db835da..398dbdd72 100644 --- a/R/data.table.R +++ b/R/data.table.R @@ -1400,7 +1400,7 @@ replace_dot_alias = function(e) { .Call(Cassign,x,irows,cols,newnames,jval) return(suppPrint(x)) } - if ((is.call(jsub) && jsub[[1L]] != "get" && is.list(jval) && !is.object(jval)) || !missingby) { + if ((is.call(jsub) && format(jsub[[1L]]) != "get" && is.list(jval) && !is.object(jval)) || !missingby) { # is.call: selecting from a list column should return list # is.object: for test 168 and 168.1 (S4 object result from ggplot2::qplot). Just plain list results should result in data.table @@ -1626,25 +1626,25 @@ replace_dot_alias = function(e) { jsub = as.call(c(quote(list), lapply(sdvars, as.name))) jvnames = sdvars } - } else if (length(as.character(jsub[[1L]])) == 1L) { # Else expect problems with + } else if (is.name(jsub[[1L]])) { # Else expect problems with # g[[ only applies to atomic input, for now, was causing #4159. be sure to eval with enclos=parent.frame() for #4612 subopt = length(jsub) == 3L && - (jsub[[1L]] == "[" || - (jsub[[1L]] == "[[" && is.name(jsub[[2L]]) && eval(call('is.atomic', jsub[[2L]]), x, parent.frame()))) && + (jsub %iscall% "[" || + (jsub %iscall% "[[" && is.name(jsub[[2L]]) && eval(call('is.atomic', jsub[[2L]]), x, parent.frame()))) && (is.numeric(jsub[[3L]]) || jsub[[3L]] == ".N") - headopt = jsub[[1L]] == "head" || jsub[[1L]] == "tail" - firstopt = jsub[[1L]] == "first" || jsub[[1L]] == "last" # fix for #2030 + headopt = jsub %iscall% c("head", "tail") + firstopt = jsub %iscall% c("first", "last") # fix for #2030 if ((length(jsub) >= 2L && jsub[[2L]] == ".SD") && (subopt || headopt || firstopt)) { if (headopt && length(jsub)==2L) jsub[["n"]] = 6L # head-tail n=6 when missing #3462 # optimise .SD[1] or .SD[2L]. Not sure how to test .SD[a] as to whether a is numeric/integer or a data.table, yet. jsub = as.call(c(quote(list), lapply(sdvars, function(x) { jsub[[2L]] = as.name(x); jsub }))) jvnames = sdvars - } else if (jsub[[1L]]=="lapply" && jsub[[2L]]==".SD" && length(xcols)) { + } else if (jsub %iscall% "lapply" && jsub[[2L]]==".SD" && length(xcols)) { deparse_ans = .massageSD(jsub) jsub = deparse_ans[[1L]] jvnames = deparse_ans[[2L]] - } else if (jsub[[1L]] == "c" && length(jsub) > 1L) { + } else if (jsub %iscall% "c" && length(jsub) > 1L) { # TODO, TO DO: raise the checks for 'jvnames' earlier (where jvnames is set by checking 'jsub') and set 'jvnames' already. # FR #2722 is just about optimisation of j=c(.N, lapply(.SD, .)) that is taken care of here. # FR #735 tries to optimise j-expressions of the form c(...) as long as ... contains @@ -1749,7 +1749,7 @@ replace_dot_alias = function(e) { GForce = FALSE } else { # Apply GForce - if (jsub[[1L]]=="list") { + if (jsub %iscall% "list") { GForce = TRUE for (ii in seq.int(from=2L, length.out=length(jsub)-1L)) { if (!.gforce_ok(jsub[[ii]], SDenv$.SDall)) {GForce = FALSE; break} @@ -1757,7 +1757,7 @@ replace_dot_alias = function(e) { } else GForce = .gforce_ok(jsub, SDenv$.SDall) if (GForce) { - if (jsub[[1L]]=="list") + if (jsub %iscall% "list") for (ii in seq_along(jsub)[-1L]) { if (is.N(jsub[[ii]])) next; # For #334 jsub[[ii]] = .gforce_jsub(jsub[[ii]], names_x) @@ -1775,7 +1775,7 @@ replace_dot_alias = function(e) { # Still do the old speedup for mean, for now nomeanopt=FALSE # to be set by .optmean() using <<- inside it oldjsub = jsub - if (jsub[[1L]]=="list") { + if (jsub %iscall% "list") { # Addressing #1369, #2949 and #1974. This used to be 30s (vs 0.5s) with 30K elements items in j, #1470. Could have been is.N() and/or the for-looped if() # jsub[[1]]=="list" so the first item of todo will always be FALSE todo = sapply(jsub, `%iscall%`, 'mean') @@ -1783,7 +1783,7 @@ replace_dot_alias = function(e) { w = which(todo) jsub[w] = lapply(jsub[w], .optmean) } - } else if (jsub[[1L]]=="mean") { + } else if (jsub %iscall% "mean") { jsub = .optmean(jsub) } if (nomeanopt) { @@ -1863,7 +1863,7 @@ replace_dot_alias = function(e) { (q[[1L]]) %chin% c("ghead", "gtail") && q3!=1) q3 else 0 } - if (jsub[[1L]] == "list"){ + if (jsub %iscall% "list"){ q3 = max(sapply(jsub, headTail_arg)) } else if (length(jsub)==3L) { q3 = headTail_arg(jsub) @@ -1965,13 +1965,9 @@ replace_dot_alias = function(e) { setalloccol(ans) # TODO: overallocate in dogroups in the first place and remove this line } -# What's the name of the top-level call in 'j'? is.name() check is required for cases like #6026 with complex calls that can't be cast to character -root_name = function(jsub) { - if (!is.call(jsub)) return("") - if (is.name(j1 <- jsub[[1L]])) return(as.character(j1)) - if (j1 %iscall% "(") return("(") # needed to prevent with=FALSE from triggering for test 1128 - "" -} +# What's the name of the top-level call in 'j'? +# NB: earlier, we used 'as.character()' but that fails for closures/builtins (#6026). +root_name = function(jsub) if (is.call(jsub)) format(jsub[[1L]]) else "" DT = function(x, ...) { #4872 old = getOption("datatable.optimize") diff --git a/R/utils.R b/R/utils.R index a78e5450f..8e14b4bbb 100644 --- a/R/utils.R +++ b/R/utils.R @@ -149,7 +149,11 @@ is_utc = function(tz) { } # very nice idea from Michael to avoid expression repetition (risk) in internal code, #4226 -"%iscall%" = function(e, f) { is.call(e) && e[[1L]] %chin% f } +"%iscall%" = function(e, f) { + if (!is.call(e)) return(FALSE) + if (is.name(e1 <- e[[1L]])) return(e %chin% f) + format(e1) %chin% f # complicated cases e.g. a closure/builtin on LHS of call; note that format() is much (e.g. 40x) slower than as.character() +} # nocov start #593 always return a data.table edit.data.table = function(name, ...) { diff --git a/inst/tests/tests.Rraw b/inst/tests/tests.Rraw index 24b4f4c60..af73fd13e 100644 --- a/inst/tests/tests.Rraw +++ b/inst/tests/tests.Rraw @@ -18425,3 +18425,8 @@ dt = data.table(a = 1L) test(2252.1, dt[, b:=2L], error = "\\[ was called on a data.table.*not data.table-aware.*':='") test(2252.2, dt[, let(b=2L)], error = "\\[ was called on a data.table.*not data.table-aware.*'let'") rm(.datatable.aware) + +# passing a function in env= doesn't trip up processing 'j', #6026 +DT=data.table(a=1:2, b=3:4) +test(2253.1, DT[, builtin(b), env=list(builtin=sum)], 7L) +test(2253.2, DT[, closure(b), env=list(closure=function(x) sum(x))], 7L) From 01ad7623d8839564f015eb92dc47e8940f7d2510 Mon Sep 17 00:00:00 2001 From: Michael Chirico Date: Thu, 28 Mar 2024 09:41:33 -0700 Subject: [PATCH 05/12] fix %iscall% --- R/utils.R | 2 +- inst/tests/tests.Rraw | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/R/utils.R b/R/utils.R index 8e14b4bbb..0eb13f463 100644 --- a/R/utils.R +++ b/R/utils.R @@ -151,7 +151,7 @@ is_utc = function(tz) { # very nice idea from Michael to avoid expression repetition (risk) in internal code, #4226 "%iscall%" = function(e, f) { if (!is.call(e)) return(FALSE) - if (is.name(e1 <- e[[1L]])) return(e %chin% f) + if (is.name(e1 <- e[[1L]])) return(e1 %chin% f) format(e1) %chin% f # complicated cases e.g. a closure/builtin on LHS of call; note that format() is much (e.g. 40x) slower than as.character() } diff --git a/inst/tests/tests.Rraw b/inst/tests/tests.Rraw index af73fd13e..f1b7d5b68 100644 --- a/inst/tests/tests.Rraw +++ b/inst/tests/tests.Rraw @@ -18429,4 +18429,5 @@ rm(.datatable.aware) # passing a function in env= doesn't trip up processing 'j', #6026 DT=data.table(a=1:2, b=3:4) test(2253.1, DT[, builtin(b), env=list(builtin=sum)], 7L) -test(2253.2, DT[, closure(b), env=list(closure=function(x) sum(x))], 7L) +test(2253.2, DT[, closure(b), env=list(closure=var)], 0.5) +test(2253.3, DT[, lambda(b), env=list(lambda=function(x) sum(x))], 7L) From ea5ce6ad0453662479298b1588f55c404fdae88c Mon Sep 17 00:00:00 2001 From: Michael Chirico Date: Thu, 28 Mar 2024 10:01:32 -0700 Subject: [PATCH 06/12] more fixes --- R/data.table.R | 4 ++-- R/utils.R | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/R/data.table.R b/R/data.table.R index 398dbdd72..a98ac9351 100644 --- a/R/data.table.R +++ b/R/data.table.R @@ -1400,7 +1400,7 @@ replace_dot_alias = function(e) { .Call(Cassign,x,irows,cols,newnames,jval) return(suppPrint(x)) } - if ((is.call(jsub) && format(jsub[[1L]]) != "get" && is.list(jval) && !is.object(jval)) || !missingby) { + if ((is.call(jsub) && !jsub %iscall% "get" && is.list(jval) && !is.object(jval)) || !missingby) { # is.call: selecting from a list column should return list # is.object: for test 168 and 168.1 (S4 object result from ggplot2::qplot). Just plain list results should result in data.table @@ -1967,7 +1967,7 @@ replace_dot_alias = function(e) { # What's the name of the top-level call in 'j'? # NB: earlier, we used 'as.character()' but that fails for closures/builtins (#6026). -root_name = function(jsub) if (is.call(jsub)) format(jsub[[1L]]) else "" +root_name = function(jsub) if (is.call(jsub)) paste(format(jsub[[1L]]), collapse = " ") else "" DT = function(x, ...) { #4872 old = getOption("datatable.optimize") diff --git a/R/utils.R b/R/utils.R index 0eb13f463..39f1262cf 100644 --- a/R/utils.R +++ b/R/utils.R @@ -152,7 +152,7 @@ is_utc = function(tz) { "%iscall%" = function(e, f) { if (!is.call(e)) return(FALSE) if (is.name(e1 <- e[[1L]])) return(e1 %chin% f) - format(e1) %chin% f # complicated cases e.g. a closure/builtin on LHS of call; note that format() is much (e.g. 40x) slower than as.character() + paste(format(e1), collapse = " ") %chin% f # complicated cases e.g. a closure/builtin on LHS of call; note that format() is much (e.g. 40x) slower than as.character() } # nocov start #593 always return a data.table From ce24d2724fa996c2836b5652b220cf743c31c34c Mon Sep 17 00:00:00 2001 From: Michael Chirico Date: Thu, 28 Mar 2024 11:17:03 -0700 Subject: [PATCH 07/12] var in earlier test was masking stats::var --- inst/tests/tests.Rraw | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/inst/tests/tests.Rraw b/inst/tests/tests.Rraw index f1b7d5b68..88bcf38f5 100644 --- a/inst/tests/tests.Rraw +++ b/inst/tests/tests.Rraw @@ -1666,8 +1666,8 @@ test(536, DT[,sum(v),by=a], data.table(a=c(1L,3L,2L),V1=c(4L,7L,10L))) # retain ans = data.table(a=1:3,V1=c(4L,10L,7L),key="a") test(537, DT[,sum(v),keyby=a], ans) test(538, DT[,sum(v),keyby="a"], ans) -var="a" -test(539, DT[,sum(v),keyby=eval(var)], ans) +byvar="a" +test(539, DT[,sum(v),keyby=eval(byvar)], ans) a=quote(a%%2L) test(540, DT[,sum(v),by=eval(a)], data.table(a=1:0,V1=c(11L,10L))) test(541, DT[,sum(v),keyby=eval(a)], data.table(a=0:1,V1=c(10L,11L),key="a")) @@ -3394,8 +3394,8 @@ test(1064, DT[integer(0), list(x2=x), by=x], output="Empty data.table (0 rows an # bug #2445 fix - := fails when subsetting yields NAs and with=FALSE X = data.table(A=1:3, B=1:6, key="A") -var <- "B" -test(1065, X[J(2:5), (var):=22L], data.table(A=rep(1:3, each=2), B=c(1L,4L,rep(22L,4)), key="A")) +jvar <- "B" +test(1065, X[J(2:5), (jvar):=22L], data.table(A=rep(1:3, each=2), B=c(1L,4L,rep(22L,4)), key="A")) # fread single unnamed colClasses f = "A,B,C,D\n1,3,5,7\n2,4,6,8\n" @@ -14014,6 +14014,7 @@ var = "b" test(1973.3, DT[K, c(var, "FOO")], c("b","FOO")) test(1973.4, DT[K, c(..var, "FOO")], ans<-data.table(b=5:6, FOO=9L)) test(1973.5, DT[K, c(var, "FOO"), with=FALSE], ans) +rm("var") # avoid masking stats::var # no error when j is supplied but inherits missingness from caller DT = data.table(a=1:3, b=4:6) @@ -18430,4 +18431,5 @@ rm(.datatable.aware) DT=data.table(a=1:2, b=3:4) test(2253.1, DT[, builtin(b), env=list(builtin=sum)], 7L) test(2253.2, DT[, closure(b), env=list(closure=var)], 0.5) -test(2253.3, DT[, lambda(b), env=list(lambda=function(x) sum(x))], 7L) +test(2253.3, DT[, closure(b), env=list(closure=stats::var)], 0.5) +test(2253.4, DT[, lambda(b), env=list(lambda=function(x) sum(x))], 7L) From a30078ad8bff6fd232eb747db20e1e53d45b4676 Mon Sep 17 00:00:00 2001 From: Michael Chirico Date: Thu, 28 Mar 2024 13:55:35 -0700 Subject: [PATCH 08/12] NEWS --- NEWS.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/NEWS.md b/NEWS.md index 7110f10e0..87b364e93 100644 --- a/NEWS.md +++ b/NEWS.md @@ -38,6 +38,8 @@ 5. `fwrite(x, row.names=TRUE)` with `x` a `matrix` writes `row.names` when present, not row numbers, [#5315](https://github.com/Rdatatable/data.table/issues/5315). Thanks to @Liripo for the report, and @ben-schwen for the fix. +6. Passing functions programmatically with `env=` works, e.g. `DT[, f(b), env = list(f=sum)]`, [#6026](https://github.com/Rdatatable/data.table/issues/6026). Thanks to @MichaelChirico for the bug report and fix. + ## NOTES 1. `transform` method for data.table sped up substantially when creating new columns on large tables. Thanks to @OfekShilon for the report and PR. The implemented solution was proposed by @ColeMiller1. From 919c262a6d534aca6c42bc9fbd636693b4c50f9d Mon Sep 17 00:00:00 2001 From: Michael Chirico Date: Thu, 28 Mar 2024 13:56:49 -0700 Subject: [PATCH 09/12] Use deparse() directly to avoid tiny overhead & make it easier to find deparse1() calls later --- R/data.table.R | 2 +- R/utils.R | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/R/data.table.R b/R/data.table.R index a98ac9351..40f75e63e 100644 --- a/R/data.table.R +++ b/R/data.table.R @@ -1967,7 +1967,7 @@ replace_dot_alias = function(e) { # What's the name of the top-level call in 'j'? # NB: earlier, we used 'as.character()' but that fails for closures/builtins (#6026). -root_name = function(jsub) if (is.call(jsub)) paste(format(jsub[[1L]]), collapse = " ") else "" +root_name = function(jsub) if (is.call(jsub)) paste(deparse(jsub[[1L]]), collapse = " ") else "" DT = function(x, ...) { #4872 old = getOption("datatable.optimize") diff --git a/R/utils.R b/R/utils.R index 39f1262cf..fd97e31ed 100644 --- a/R/utils.R +++ b/R/utils.R @@ -152,7 +152,7 @@ is_utc = function(tz) { "%iscall%" = function(e, f) { if (!is.call(e)) return(FALSE) if (is.name(e1 <- e[[1L]])) return(e1 %chin% f) - paste(format(e1), collapse = " ") %chin% f # complicated cases e.g. a closure/builtin on LHS of call; note that format() is much (e.g. 40x) slower than as.character() + paste(deparse(e1), collapse = " ") %chin% f # complicated cases e.g. a closure/builtin on LHS of call; note that format() is much (e.g. 40x) slower than as.character() } # nocov start #593 always return a data.table From 0d882ad91ee171a13148ee2c37ed03f51b69b789 Mon Sep 17 00:00:00 2001 From: Michael Chirico Date: Mon, 29 Jul 2024 12:22:01 -0700 Subject: [PATCH 10/12] discourage f=sum, encourage f="sum" --- NEWS.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/NEWS.md b/NEWS.md index 6b14554d0..cc2f782f5 100644 --- a/NEWS.md +++ b/NEWS.md @@ -76,7 +76,7 @@ 15. `fread()` is more careful about detecting that a file is compressed in bzip2 format, [#6304](https://github.com/Rdatatable/data.table/issues/6304). In particular, we also check the 4th byte is a digit; in rare cases, a legitimate uncompressed CSV file could match 'BZh' as the first 3 bytes. We think an uncompressed CSV file matching 'BZh[1-9]' is all the more rare and unlikely to be encountered in "real" examples. Other formats (zip, gzip) are friendly enough to use non-printable characters in their magic numbers. Thanks @grainnemcguire for the report and @MichaelChirico for the fix. -13. Passing functions programmatically with `env=` works, e.g. `DT[, f(b), env = list(f=sum)]`, [#6026](https://github.com/Rdatatable/data.table/issues/6026). Thanks to @MichaelChirico for the bug report and fix. +13. Passing functions programmatically with `env=` doesn't produce an opaque error, e.g. `DT[, f(b), env = list(f=sum)]`, [#6026](https://github.com/Rdatatable/data.table/issues/6026). Note that it's much better to pass functions like `f="sum"` instead. Thanks to @MichaelChirico for the bug report and fix. ## NOTES From df9c0858f57eea87216a7519cb7e516b13a0828d Mon Sep 17 00:00:00 2001 From: Michael Chirico Date: Mon, 29 Jul 2024 12:23:15 -0700 Subject: [PATCH 11/12] Also catch pkg:::fun --- R/utils.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R/utils.R b/R/utils.R index a6f0f2fad..cf23609ee 100644 --- a/R/utils.R +++ b/R/utils.R @@ -155,7 +155,7 @@ is_utc = function(tz) { `%iscall%` = function(e, f) { if (!is.call(e)) return(FALSE) if (is.name(e1 <- e[[1L]])) return(e1 %chin% f) - if (e1 %iscall% '::') return(e1[[3L]] %chin% f) + if (e1 %iscall% c('::', ':::')) return(e1[[3L]] %chin% f) paste(deparse(e1), collapse = " ") %chin% f # complicated cases e.g. a closure/builtin on LHS of call; note that format() is much (e.g. 40x) slower than deparse() } From 6c4d0ad2feb73074174d3a4bb1836cb0bc3f05ec Mon Sep 17 00:00:00 2001 From: Michael Chirico Date: Mon, 29 Jul 2024 12:24:21 -0700 Subject: [PATCH 12/12] test for ':::' too --- inst/tests/tests.Rraw | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/inst/tests/tests.Rraw b/inst/tests/tests.Rraw index 553b19e0f..124e4d817 100644 --- a/inst/tests/tests.Rraw +++ b/inst/tests/tests.Rraw @@ -18820,4 +18820,5 @@ DT=data.table(a=1:2, b=3:4) test(2275.1, DT[, builtin(b), env=list(builtin=sum)], 7L) test(2275.2, DT[, closure(b), env=list(closure=var)], 0.5) test(2275.3, DT[, closure(b), env=list(closure=stats::var)], 0.5) -test(2275.4, DT[, lambda(b), env=list(lambda=function(x) sum(x))], 7L) +test(2275.4, DT[, closure(b), env=list(closure=stats:::var)], 0.5) +test(2275.5, DT[, lambda(b), env=list(lambda=function(x) sum(x))], 7L)