Missing both i and j

CRAN_Release.cmd

@@ -410,7 +410,8 @@ biocLite()   # keep repeating until returns with nothing left to do.   Note: hug
 
 avail = available.packages(repos=biocinstallRepos())   # includes CRAN at the end from getOption("repos")
 deps = tools::package_dependencies("data.table", db=avail, which="most", reverse=TRUE, recursive=FALSE)[[1]]
-exclude = c("TCGAbiolinks")   # takes loo long: https://github.com/BioinformaticsFMRP/TCGAbiolinks/issues/240
+exclude = c("TCGAbiolinks", # takes loo long: https://github.com/BioinformaticsFMRP/TCGAbiolinks/issues/240
+            "facopy")       # fails for over a year and isn't getting fixed:  Error : object ‘setting.graph.attributes’ is not exported by 'namespace:DOSE'
 deps = deps[-match(exclude, deps)]
 table(avail[deps,"Repository"])
 length(deps)

NEWS.md

@@ -44,6 +44,13 @@
 
 11. `split` data.table method will now preserve attributes, closes [#2047](https://github.com/Rdatatable/data.table/issues/2047). Thanks to @caneff for reporting.
 
+12. `DT[i,j]` now retains user-defined and inherited attributes, [#995](https://github.com/Rdatatable/data.table/issues/995); e.g.
+    ```R
+    attr(datasets::BOD,"reference")                     # "A1.4, p. 270"
+    attr(as.data.table(datasets::BOD)[2],"reference")   # was NULL now "A1.4, p. 270"
+    ```
+    If a superclass defines attributes that may not be valid after a `[` subset then the superclass should implement its own `[` method to manage those after calling `NextMethod()`.
+
 #### BUG FIXES
 
 1. Providing an `i` subset expression when attempting to delete a column correctly failed with helpful error, but when the column was missing too created a new column full of `NULL` values, [#3089](https://github.com/Rdatatable/data.table/issues/3089). Thanks to Michael Chirico for reporting.
@@ -60,19 +67,13 @@
 
 7. `split.data.table()` failed if `DT` had a factor column named `"x"`, [#3151](https://github.com/Rdatatable/data.table/issues/3151). Thanks to @tdeenes for reporting and fixing.
 
-8. `DT[i,j]` now retains user-defined or inherited attributes; e.g.
-    ```R
-    attr(datasets::BOD,"reference")                     # "A1.4, p. 270"
-    attr(as.data.table(datasets::BOD)[2],"reference")   # was NULL now "A1.4, p. 270"
-    ```
-
-9. `fsetequal` now handles properly datasets having last column a character, closes [#2318](https://github.com/Rdatatable/data.table/issues/2318). Thanks to @pschil and @franknarf1 for reporting.
+8. `fsetequal` now handles properly datasets having last column a character, closes [#2318](https://github.com/Rdatatable/data.table/issues/2318). Thanks to @pschil and @franknarf1 for reporting.
 
-10. `DT[..., .SDcols=integer(0L)]` could fail, [#3185](https://github.com/Rdatatable/data.table/issues/3185). An empty `data.table` is now returned correctly.
+9. `DT[..., .SDcols=integer(0L)]` could fail, [#3185](https://github.com/Rdatatable/data.table/issues/3185). An empty `data.table` is now returned correctly.
 
-11. `as.data.table.default` method will now always copy its input, closes [#3230](https://github.com/Rdatatable/data.table/issues/3230). Thanks to @NikdAK for reporting.
+10. `as.data.table.default` method will now always copy its input, closes [#3230](https://github.com/Rdatatable/data.table/issues/3230). Thanks to @NikdAK for reporting.
 
-12. `DT[..., .SDcols=integer()]` failed with `.SDcols is numeric but has both +ve and -ve indices`, [#1789](https://github.com/Rdatatable/data.table/issues/1789) and [#3185](https://github.com/Rdatatable/data.table/issues/3185). It now functions as `.SDcols=character()` has done and creates an empty `.SD`. Thanks to Gabor Grothendieck and Hugh Parsonage for reporting. A related issue with empty `.SDcols` was fixed in development before release thanks to Kun Ren's testing, [#3211](https://github.com/Rdatatable/data.table/issues/3211).
+11. `DT[..., .SDcols=integer()]` failed with `.SDcols is numeric but has both +ve and -ve indices`, [#1789](https://github.com/Rdatatable/data.table/issues/1789) and [#3185](https://github.com/Rdatatable/data.table/issues/3185). It now functions as `.SDcols=character()` has done and creates an empty `.SD`. Thanks to Gabor Grothendieck and Hugh Parsonage for reporting. A related issue with empty `.SDcols` was fixed in development before release thanks to Kun Ren's testing, [#3211](https://github.com/Rdatatable/data.table/issues/3211).
 
 #### NOTES
 

R/data.table.R

@@ -243,8 +243,13 @@ chmatch2 <- function(x, table, nomatch=NA_integer_) {
   }
   .global$print=""
   if (missing(i) && missing(j)) {
-    if (!is.null(names(sys.call())))   # not using nargs() as it considers DT[,] to have 3 arguments, #3163
-      stop("When i and j are both missing, no other argument should be used. Empty [] is useful after := to have the result displayed; e.g. DT[,col:=val][]")
+    tt_isub = substitute(i)
+    tt_jsub = substitute(j)
+    if (!is.null(names(sys.call())) &&  # not relying on nargs() as it considers DT[,] to have 3 arguments, #3163
+        tryCatch(!is.symbol(tt_isub), error=function(e)TRUE) &&   # a symbol that inherits missingness from caller isn't missing for our purpose; test 1974
+        tryCatch(!is.symbol(tt_jsub), error=function(e)TRUE)) {
+      warning("i and j are both missing so ignoring the other arguments")
+    }
     return(x)
   }
   if (!mult %chin% c("first","last","all")) stop("mult argument can only be 'first','last' or 'all'")
@@ -268,7 +273,7 @@ chmatch2 <- function(x, table, nomatch=NA_integer_) {
   if ((isTRUE(which)||is.na(which)) && !missing(j)) stop("which==",which," (meaning return row numbers) but j is also supplied. Either you need row numbers or the result of j, but only one type of result can be returned.")
   if (!is.na(nomatch) && is.na(which)) stop("which=NA with nomatch=0 would always return an empty vector. Please change or remove either which or nomatch.")
   if (!with && missing(j)) stop("j must be provided when with=FALSE")
-  if (missing(i) && !missing(on)) stop("i must be provided when on= is provided")
+  if (missing(i) && !missing(on)) warning("ignoring on= because it is only relevant to i but i is not provided")
   if (!missing(keyby)) {
     if (!missing(by)) stop("Provide either 'by' or 'keyby' but not both")
     by=bysub=substitute(keyby)

inst/tests/tests.Rraw

@@ -12490,10 +12490,10 @@ test(1948.15, DT[i, on = 1L], error = "'on' argument should be a named atomic ve
 
 # helpful error when on= is provided but not i, rather than silently ignoring on=
 DT = data.table(A=1:3)
-test(1949.1, DT[,,on=A], error="When i and j are both missing, no other argument should be used.")
-test(1949.2, DT[,1,on=A], error="i must be provided when on= is provided")
-test(1949.3, DT[on=A], error="When i and j are both missing, no other argument should be used.")
-test(1949.4, DT[,on=A], error="When i and j are both missing, no other argument should be used.")
+test(1949.1, DT[,,on=A], DT, warning="i and j are both missing so ignoring the other arguments")
+test(1949.2, DT[,1,on=A], DT, warning="ignoring on= because it is only relevant to i but i is not provided")
+test(1949.3, DT[on=A], DT, warning="i and j are both missing so ignoring the other arguments")
+test(1949.4, DT[,on=A], DT, warning="i and j are both missing so ignoring the other arguments")
 test(1949.5, DT[1,,with=FALSE], error="j must be provided when with=FALSE")
 test(1949.6, DT[], output="A.*1.*2.*3")   # no error
 test(1949.7, DT[,], output="A.*1.*2.*3")  # no error, #3163
@@ -13216,6 +13216,13 @@ 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)
 
+# no error when j is supplied but inherits missingness from caller
+DT = data.table(a=1:3, b=4:6)
+f = function(cols) DT[,cols]
+test(1974.1, f(), output="a.*b.*3:.*6")
+f = function(cols) DT[,cols,with=FALSE]
+test(1974.2, f(), output="a.*b.*3:.*6")
+
 
 ###################################
 #  Add new tests above this line  #

src/subset.c

@@ -244,9 +244,14 @@ SEXP subsetDT(SEXP x, SEXP rows, SEXP cols) {
   }
 
   int overAlloc = checkOverAlloc(GetOption(install("datatable.alloccol"), R_NilValue));
-  SEXP ans = PROTECT(allocVector(VECSXP, LENGTH(cols)+overAlloc)); nprotect++;  // just do alloc.col directly, eventually alloc.col can be deprecated.
-  copyMostAttrib(x, ans);  // other than R_NamesSymbol, R_DimSymbol and R_DimNamesSymbol
-               // so includes row.names (oddly, given other dims aren't) and "sorted", dealt with below
+  SEXP ans = PROTECT(allocVector(VECSXP, LENGTH(cols)+overAlloc)); nprotect++;  // doing alloc.col directly here; eventually alloc.col can be deprecated.
+
+  // user-defined and superclass attributes get copied as from v1.12.0
+  copyMostAttrib(x, ans);
+  // most means all except R_NamesSymbol, R_DimSymbol and R_DimNamesSymbol
+  // includes row.names (oddly, given other dims aren't) and "sorted" dealt with below
+  // class is also copied here which retains superclass name in class vector as has been the case for many years; e.g. tests 1228.* for #5296
+
   SET_TRUELENGTH(ans, LENGTH(ans));
   SETLENGTH(ans, LENGTH(cols));
   int ansn;