refactor: add extractByIndex method

R/MsBackend.R

@@ -15,6 +15,7 @@
 #' @aliases dataStorageBasePath,MsBackendMzR-method
 #' @aliases dataStorageBasePath<-
 #' @aliases dataStorageBasePath<-,MsBackendMzR-method
+#' @aliases extractByIndex
 #' @aliases msLeveL<-,MsBackend-method
 #'
 #' @description
@@ -223,7 +224,9 @@
 #'   allowed. Parameter `i` should support `integer` indices and `logical`
 #'   and should throw an error if `i` is out of bounds. The
 #'   `MsCoreUtils::i2index` could be used to check the input `i`.
-#'   For `i = integer()` an empty backend should be returned.
+#'   For `i = integer()` an empty backend should be returned. Implementation
+#'   of this method is optional, as the default calls the `extractByIndex()`
+#'   method (which has to be implemented as the main subsetting method).
 #'
 #' - `$`, `$<-`: access or set/add a single spectrum variable (column) in the
 #'   backend. Using a `value` of `NULL` should allow deleting the specified
@@ -337,8 +340,7 @@
 #'   that can result in implementations of `[` being not found by R (which
 #'   can happen sometimes in parallel processing using the [SnowParam()]). This
 #'   method is used internally by `Spectra` to extract/subset its backend.
-#'   Implementation is optional, as the default implementation for `MsBackend`
-#'   will use `[`.
+#'   Implementation of this method is mandatory.
 #'
 #' - `filterAcquisitionNum()`: filters the object keeping only spectra matching
 #'   the provided acquisition numbers (argument `n`). If `dataOrigin` or
@@ -1117,14 +1119,13 @@ setMethod("dropNaSpectraVariables", "MsBackend", function(object) {
 #'
 #' @export
 setMethod("extractByIndex", c("MsBackend", "ANY"), function(object, i) {
-    object[i = i]
+    stop("'extractByIndex' not implemented for ", class(object), ".")
 })
 
 #' @rdname MsBackend
 #'
 #' @export
 setMethod("extractByIndex", c("MsBackend", "missing"), function(object, i) {
-    message("extractByIndex,MsBackend,missing")
     object
 })
 
@@ -1858,7 +1859,7 @@ setMethod("tic", "MsBackend", function(object, initial = TRUE) {
 #'
 #' @export
 setMethod("[", "MsBackend", function(x, i, j, ..., drop = FALSE) {
-    stop("Not implemented for ", class(x), ".")
+    extractByIndex(x, i2index(i, length = length(x)))
 })
 
 #' @exportMethod $

R/MsBackendCached.R

@@ -437,7 +437,7 @@ setMethod("show", "MsBackendCached", function(object) {
     cat(class(object), "with", n, "spectra\n")
     if (n) {
         idx <- unique(c(1L:min(6L, n), max(1L, n-5L):n))
-        spd <- spectraData(object[idx, ],
+        spd <- spectraData(extractByIndex(object, idx),
                            c("msLevel", "precursorMz", "polarity"))
         if (!length(rownames(spd)))
             rownames(spd) <- idx

R/MsBackendDataFrame.R

@@ -574,5 +574,5 @@ setMethod("filterAcquisitionNum", "MsBackendDataFrame",
                              "acquisition number(s) for sub-setting")
     sel_file <- .sel_file(object, dataStorage, dataOrigin)
     sel_acq <- acquisitionNum(object) %in% n & sel_file
-    object[sel_acq | !sel_file]
+    extractByIndex(object, which(sel_acq | !sel_file))
 })

inst/test_backends/test_MsBackend/test_spectra_subsetting.R

@@ -49,6 +49,24 @@ test_that("[", {
     res <- be[integer()]
     expect_s4_class(res, class(be)[1L])
     expect_true(length(res) == 0L)
+
+    ## logical
+    l <- rep(FALSE, length(be))
+    l[sample(seq_along(l), floor(length(l) / 2))] <- TRUE
+    res <- be[l]
+    expect_true(validObject(res))
+    expect_true(length(res) == sum(l))
+    expect_equal(res, be[which(l)])
+})
+
+#' extractByIndex. Uses [ if not implemented
+test_that("extractByIndex", {
+    i <- sample(seq_along(be), floor(length(be) / 2))
+    res <- extractByIndex(be, i)
+    expect_true(validObject(res))
+    expect_equal(length(res), length(i))
+    expect_equal(msLevel(res), msLevel(be)[i])
+    expect_equal(rtime(res), rtime(be)[i])
 })
 
 #' dropNASpectraVariables: only for not read-only

tests/testthat/test_MsBackend.R

@@ -57,8 +57,6 @@ test_that("MsBackend methods throw errors", {
     expect_error(dm$a, "implemented for")
     expect_error(dm$a <- "a", "implemented for")
     expect_error(extractByIndex(dm, 1), "implemented for")
-
-    expect_equal(extractByIndex(dm), dm)
 })
 
 test_that("reset,MsBackend works", {

vignettes/MsBackend.Rmd

@@ -563,35 +563,39 @@ additionally available variables and the `columns` parameter of the
 (in addition to the required `"mz"` and `"intensity"` variables).
 
 
-### `[`
-
-The `[` method allows to subset `MsBackend` objects. This operation is expected
-to reduce a `MsBackend` object to the selected spectra. The method should
-support to subset by indices or logical vectors and should also support
-duplicating elements (i.e. when duplicated indices are used) as well as to
-subset in arbitrary order. An error should be thrown if indices are out of
-bounds, but the method should also support returning an empty backend with
-`[integer()]`. Note that the `MsCoreUtils::i2index` function can be used to
+### `extractByIndex()` and `[`
+
+The `extractByIndex()` and `[` methods allows to subset `MsBackend` objects.
+This operation is expected to reduce a `MsBackend` object to the selected
+spectra. These methods must also support duplication (e.g. `[c(1, 1, 1)]` and
+extraction in any arbitrary order (e.g. `[c(3, 1, 5, 3)]`). While both methods
+subset the object, `extractByIndex()` only supports to subset with an `integer`
+index, while `[`, to be compliant with the base R implementation, should support
+to subset by indices or logical vectors. An error should be thrown if indices
+are out of bounds, but the method should also support returning an empty backend
+with `[integer()]`. Note that the `MsCoreUtils::i2index` function can be used to
 check for correct input (and convert the input to an `integer` index).
 
-Below we implement a possible `[` for our test backend class. We ignore the
-parameters `j` from the definition of the `[` generic, since we treat our data
-to be one-dimensional (with each spectrum being one element).
+The `extractByIndex()` method is used by the data operation and analysis methods
+on `Spectra` objects, while the `[` is intended to be used by the end user (if
+needed). Below we implement `extractByIndex()` for our backend:
 
 ```{r}
-setMethod("[", "MsBackendTest", function(x, i, j, ..., drop = FALSE) {
-    i <- MsCoreUtils::i2index(i, length = length(x))
-    x@spectraVars <- x@spectraVars[i, ]
-    x@mz <- x@mz[i]
-    x@intensity <- x@intensity[i]
-    x
+setMethod("extractByIndex", c("MsBackendTest", "ANY"), function(object, i) {
+    object@spectraVars <- object@spectraVars[i, ]
+    object@mz <- object@mz[i]
+    object@intensity <- object@intensity[i]
+    object
 })
 ```
 
+The `[` does not need to be defined because a default implementation for
+the base `MsBackend` exists.
+
 We can now subset our backend to the last two spectra.
 
 ```{r}
-a <- be[2:3]
+a <- extractByIndex(be, 2:3)
 spectraData(a)
 ```
 
@@ -602,28 +606,6 @@ a <- be[c(2, 2, 2)]
 spectraData(a)
 ```
 
-In addition to the `[` method it is also suggested to implement a
-`extractByIndex()` method. Similar to `[`, this method should extract elements
-from, or subset, a backend, but it expects an integer vector with the indices as
-second parameter. Hence, this method does not require conversion of
-e.g. `logical` to `integer`. Also, this method helps avoiding namespace issues
-sometimes encountered with parallel processing using the `SnowParam` when the
-implementation of `[` for certain backends are not found.
-
-Data analysis methods on `Spectra` objects will use this method for most
-operations. Implementation of this method is optional, since the default
-implementation for `MsBackend` will fall back to `[`. Below we implement this
-method for our example backend.
-
-```{r}
-setMethod("extractByIndex", c("MsBackendTest", "ANY"), function(object, i) {
-    x@spectraVars <- x@spectraVars[i, ]
-    x@mz <- x@mz[i]
-    x@intensity <- x@intensity[i]
-    x
-})
-```
-
 
 ### `backendMerge()`