j = <expr>:<expr> is now with=TRUE with at least one call expr (#4460)

* Use with=TRUE for call:call j expressions * simplify test, add more checks * improve NEWS * NEWS grammar --------- Co-authored-by: Michael Chirico <[email protected]>
Rdatatable · Sep 7, 2024 · db3cbb4 · db3cbb4
1 parent 3260cb7
commit db3cbb4
Show file tree

Hide file tree

Showing 3 changed files with 16 additions and 4 deletions.
diff --git a/NEWS.md b/NEWS.md
@@ -53,6 +53,8 @@ rowwiseDT(
     # [1] "V1" "b" "c"
     ```
 
+5. Queries like `DT[, min(x):max(x)]` now work as expected, i.e. the same as `DT[, seq(min(x), max(x))]` or `with(DT, min(x):max(x))`, [#2069](https://github.com/Rdatatable/data.table/issues/2069). Shorthand like `DT[, a:b]` meaning "select from columns `a` through `b`" still works. Thanks to @franknarf1 for reporting and @jangorecki for the fix.
+
 ## NOTES
 
 1. Tests run again when some Suggests packages are missing, [#6411](https://github.com/Rdatatable/data.table/issues/6411). Thanks @aadler for the note and @MichaelChirico for the fix.

diff --git a/R/data.table.R b/R/data.table.R
@@ -244,10 +244,10 @@ replace_dot_alias = function(e) {
   if (!missing(j)) {
     jsub = replace_dot_alias(jsub)
     root = root_name(jsub)
-    if (root == ":" ||
-        (root %chin% c("-","!") && jsub[[2L]] %iscall% '(' && jsub[[2L]][[2L]] %iscall% ':') ||
-        ( (!length(av<-all.vars(jsub)) || all(startsWith(av, ".."))) &&
-          root %chin% c("","c","paste","paste0","-","!") &&
+    if ((root == ":" && !is.call(jsub[[2L]]) && !is.call(jsub[[3L]])) ||                        ## x[, V1:V2]; but not x[, (V1):(V2)] #2069
+        (root %chin% c("-","!") && jsub[[2L]] %iscall% '(' && jsub[[2L]][[2L]] %iscall% ':') || ## x[, !(V8:V10)]
+        ( (!length(av<-all.vars(jsub)) || all(startsWith(av, ".."))) &&                         ## x[, "V1"]; x[, ..v]
+          root %chin% c("","c","paste","paste0","-","!") &&                                     ## x[, c("V1","V2")]; x[, paste("V",1:2,sep="")]; x[, paste0("V",1:2)]
           missingby )) {   # test 763. TODO: likely that !missingby iff with==TRUE (so, with can be removed)
       # When no variable names (i.e. symbols) occur in j, scope doesn't matter because there are no symbols to find.
       # If variable names do occur, but they are all prefixed with .., then that means look up in calling scope.

diff --git a/inst/tests/tests.Rraw b/inst/tests/tests.Rraw
@@ -19155,3 +19155,13 @@ for (opt in c(0, 1, 2)) {
        names(M[, c(mpg[1], list(mpg, b=hp), c(lapply(.SD, mean))), by="cyl", .SDcols=c("vs", "am")]),
        c("cyl", "V1", "V2", "b", "vs", "am"))
 }
+
+# Confusing behavior with DT[, min(var):max(var)] #2069
+DT = data.table(t = c(2L, 1L, 3L))
+test(2284.1, DT[, min(t):max(t)], with(DT, min(t):max(t)))
+test(2284.2, DT[, 1:max(t)], with(DT, 1:max(t)))
+test(2284.3, DT[, max(t):1], with(DT, max(t):1))
+test(2284.4, DT[, 1:t[1L]], with(DT, 1:t[1L]))
+test(2284.5, DT[, t[2L]:1], with(DT, t[2L]:1))
+test(2284.6, DT[1L, t:max(t)], with(DT[1L], t:max(t)))
+test(2284.7, DT[1L, min(t):t], with(DT[1L], min(t):t))