Extend not-in-scope error for when it might be an integer column (#3589)

NEWS.md

@@ -140,6 +140,7 @@
 
 8. Historically, `dcast` and `melt` were built as enhancements to `reshape2`'s own `dcast`/`melt`. We removed dependency on `reshape2` in v1.9.6 but maintained some backward compatibility. As that package has been deprecated since December 2017, we have now formally completed the split from `reshape2` by removing some last vestiges, [#3549](https://github.com/Rdatatable/data.table/issues/3549). We thank the `reshape2` authors for their original inspiration for these functions.
 
+9. `DT[col]` where `col` is a column containing row numbers of itself to select, now suggests the correct syntax (`DT[(col)]` or `DT[DT$col]`), [#697](https://github.com/Rdatatable/data.table/issues/697). This expands the message introduced in [#1884](https://github.com/Rdatatable/data.table/issues/1884) for the case where `col` is type `logical` and `DT[col==TRUE]` is suggested.
 
 ### Changes in [v1.12.2](https://github.com/Rdatatable/data.table/milestone/14?closed=1)  (07 Apr 2019)
 

R/data.table.R

@@ -452,10 +452,11 @@ replace_dot_alias <- function(e) {
       if (inherits(i,"try-error")) {
         # must be "not found" since isub is a mere symbol
         col = try(eval(isub, x), silent=TRUE)  # is it a column name?
-        if (identical(typeof(col),"logical"))
-          stop(as.character(isub)," is not found in calling scope but it is a column of type logical. If you wish to select rows where that column is TRUE, either wrap the symbol with '()' or use ==TRUE to be clearest to readers of your code.")
-        else
-          stop(as.character(isub)," is not found in calling scope and it is not a column of type logical. When the first argument inside DT[...] is a single symbol, data.table looks for it in calling scope.")
+        msg = if (inherits(col,"try-error")) " and it is not a column name either."
+              else paste0(" but it is a column of type ", typeof(col),". If you wish to select rows where that column contains TRUE",
+                          ", or perhaps that column contains row numbers of itself to select, try DT[(col)], DT[DT$col], or DT[col==TRUE] is particularly clear and is optimized.")
+        stop(as.character(isub), " is not found in calling scope", msg,
+             " When the first argument inside DT[...] is a single symbol (e.g. DT[var]), data.table looks for var in calling scope.")
       }
     }
     if (restore.N) {
@@ -633,7 +634,7 @@ replace_dot_alias <- function(e) {
       }
       # TO DO: TODO: Incorporate which_ here on DT[!i] where i is logical. Should avoid i = !i (above) - inefficient.
       # i is not a data.table
-      if (!is.logical(i) && !is.numeric(i)) stop("i has not evaluated to logical, integer or double")
+      if (!is.logical(i) && !is.numeric(i)) stop("i has evaluated to type ", typeof(i), ". Expecting logical, integer or double.")
       if (is.logical(i)) {
         if (length(i)==1L  # to avoid unname copy when length(i)==nrow (normal case we don't want to slow down)
           && isTRUE(unname(i))) { irows=i=NULL }  # unname() for #2152 - length 1 named logical vector.

inst/tests/tests.Rraw

@@ -11036,11 +11036,18 @@ test(1772.2, split(DT, by = c('a', 'b'), sorted = TRUE),
 if (exists("A")) rm(A)
 if (exists("B")) rm(B)
 if (exists("NOTEXIST")) rm(NOTEXIST)
-DT <- data.table(A = c(FALSE, TRUE), B = 1:2)
-test(1773.1, DT[A], error = "not found in calling scope.*wrap the symbol with")
-test(1773.2, DT[B], error = "not found in calling scope and it is not a column of type logical")
-test(1773.3, DT[NOTEXIST], error = "not found in calling scope and it is not a column of type logical")
-test(1773.4, DT[(A)], DT[2])
+if (exists("MyCol")) rm(MyCol)
+DT <- data.table(A = c(FALSE, TRUE), B = 2:1, C=c(2,3), MyCol=c(2,2))
+test(1773.01, DT[A], error = "A is not found in calling scope but it is a column of type logical.*==TRUE.*When the first argument")
+test(1773.02, DT[B], error = "B is not found in calling scope but it is a column of type integer.*DT\\[\\(col\\)\\].*When the first argument")   # 697
+test(1773.03, DT[C], error = "i has evaluated to type closure. Expecting logical, integer or double")  # C picks up stats::C in calling scope
+test(1773.04, DT[MyCol], error="MyCol is not found in calling scope but it is a column of type double.*DT\\[\\(col\\)\\].*When the first argument")
+test(1773.05, DT[NOTEXIST], error = "NOTEXIST is not found in calling scope and it is not a column name either. When the first argument")
+test(1773.06, DT[(A)], DT[2])
+test(1773.07, DT[A==TRUE], DT[2])
+test(1773.08, DT[(B)], data.table(A=c(TRUE,FALSE), B=1:2, C=c(3,2), MyCol=2))
+test(1773.09, DT[(MyCol)], data.table(A=c(TRUE,TRUE), B=INT(1,1), C=c(3,3), MyCol=2))
+test(1773.10, DT[(C)], data.table(A=c(TRUE,NA), B=c(1L,NA), C=c(3,NA), MyCol=c(2,NA)))
 
 # New as.data.table.array method in v1.10.5
 set.seed(1L)
@@ -13399,7 +13406,7 @@ test(1967.48, x[ , b, .SDcols = 'a'], 6:10,
 test(1967.49, x[ , list(5) := 6], error = 'LHS of := must be a symbol')
 test(1967.50, x[ , 1 + 3i := 6], error = "LHS of := isn't column names")
 test(1967.511, x[ , .(5L), by = .EACHI, mult = 'all'], error='logical error. i is not data.table')
-test(1967.512, x[1+3i], error='i has not evaluated to logical')
+test(1967.512, x[1+3i], error='i has evaluated to type complex. Expecting logical, integer or double')
 test(1967.521, x[1:2, by=a], x[1:2,], warning="Ignoring by= because j= is not supplied")
 test(1967.522, x[, by=a],    x,       warning=c("Ignoring by= because j= is not supplied","i and j are both missing.*upgraded to error in future"))
 test(1967.523, x[by=a],      x,       warning=c("Ignoring by= because j= is not supplied","i and j are both missing.*upgraded to error in future"))
@@ -14856,7 +14863,6 @@ test(2050.4, rbind(DT[0], DT[0])[,levels(f)], c("a","b","c"))  # now ok again (o
 test(2050.5, rbindlist(list(DT[0], DT[0]))[,levels(f)], c("a","b","c"))  # now ok again
 test(2050.6, rbind(DT[1], data.table(f=factor(letters[10:11]))[0])[,levels(f)], c("a","b","c","j","k")) # now includes "j","k" again
 
-
 ###################################
 #  Add new tests above this line  #
 ###################################