Tix removeReporters,OnDiskMSnExp

o Fix the removeReporters,OnDiskMSnExp method (see issue #161). o Add unit test.
lgatto · Oct 10, 2016 · 8d7ec82 · 8d7ec82
1 parent 321ff72
commit 8d7ec82
Show file tree

Hide file tree

Showing 5 changed files with 48 additions and 14 deletions.
diff --git a/R/methods-MSnExp.R b/R/methods-MSnExp.R
@@ -261,6 +261,9 @@ setMethod("removeReporters", "MSnExp",
                    verbose = isMSnbaseVerbose()) {
               if (is.null(reporters))
                   return(object)
+              ## Throw an error if only MS1 spectra present.
+              if (all(msLevel(object) == 1))
+                  stop("No reporters to remove for MS1 spectra.")
               removeReporters_MSnExp(object, reporters, clean, verbose)
           })
 

diff --git a/R/methods-OnDiskMSnExp.R b/R/methods-OnDiskMSnExp.R
@@ -825,23 +825,33 @@ setMethod("estimateNoise", "OnDiskMSnExp",
               return(res)
           })
 
-setMethod("removeReporters", "OnMSnExp",
+############################################################
+## removeReporters
+##
+## Adds the removeReporters Spectrum processing step to the lazy processing
+## queue
+setMethod("removeReporters", "OnDiskMSnExp",
           function(object, reporters = NULL, clean = FALSE,
                    verbose = isMSnbaseVerbose()) {
               if (is.null(reporters)) {
                   return(object)
               } else {
+                  ## Check if we've got msLevel > 1.
+                  if (all(msLevel(object) == 1))
+                      stop("No MS level > 1 spectra present! Reporters can",
+                           " only be removed from spectra >= 2!")
                   ps <- ProcessingStep("removeReporters",
                                        list(reporters = reporters,
-                                            clean = clean))
-                  x@spectraProcessingQueue <- c(x@spectraProcessingQueue,
-                                                list(ps))
+                                            clean = clean,
+                                            suppressWarnings = TRUE))
+                  object@spectraProcessingQueue <- c(object@spectraProcessingQueue,
+                                                     list(ps))
                   repname <- names(reporters)
-                  x@processingData@processing <- c(x@processingData@processing,
+                  object@processingData@processing <- c(object@processingData@processing,
                                                    paste("Removed",
                                                          repname,
                                                          "reporter ions ",
                                                          date()))
-                  return(x)
+                  return(object)
               }
           })
diff --git a/R/methods-Spectrum.R b/R/methods-Spectrum.R
@@ -249,10 +249,17 @@ setMethod("smooth", "Spectrum",
           })
 
 setMethod("removeReporters", "Spectrum",
-          function(object, reporters = NULL, clean = FALSE) {
+          function(object, reporters = NULL, clean = FALSE, ...) {
               if (msLevel(object) > 1)
                   return(removeReporters_Spectrum2(object, reporters, clean))
-              stop("No reporters to remove for MS1 spectra.")
+              ## stop("No reporters to remove for MS1 spectra.")
+              ## Instead of stopping we show a (conditional) warning
+              ## See also issue #161
+              dots <- list(...)
+              if (!((length(dots$suppressWarnings) > 0) && dots$suppressWarnings))
+                  warning("No reporters to remove for MS1 spectra.")
+              ## Return the Spectrum as-is for MS1
+              return(object)
           })
 
 setMethod("isEmpty", "Spectrum",

diff --git a/man/MSnExp-class.Rd b/man/MSnExp-class.Rd
@@ -23,7 +23,7 @@
 \alias{quantify,MSnExp,character-method}
 \alias{removePeaks,MSnExp-method}
 \alias{removeReporters,MSnExp-method}
-\alias{removeReporters,OnMSnExp-method}
+\alias{removeReporters,OnDiskMSnExp-method}
 \alias{smooth,MSnExp-method}
 \alias{addIdentificationData,MSnExp,character-method}
 \alias{addIdentificationData,MSnExp,mzIDClasses-method}
@@ -224,7 +224,7 @@
     \item{isolationWindow}{\code{signature(object = "MSnExp", ...)}:
       Returns the isolation window offsets for the MS2 spectra. See
       \code{\link[mzR]{isolationWindow}} for details. }
-    
+
     \item{trimMz}{\code{signature(object = "MSnExp")}: Trims the MZ
       range of all the spectra of the \code{MSnExp} instance. See
       \code{\link{trimMz}} documentation for more details and
@@ -247,11 +247,11 @@
       expected to be in the same, albeit possibly different modes, it is
       advised to assign the majority result for MS1 and MS2 spectra,
       rather than results for individual spectra. See an example below.
-    }    
-  }  
+    }
+  }
 
   Filtering functions:
-  
+
   \describe{
 
     \item{filterRt}{\code{signature(object = "MSnExp", rt = "numeric",

diff --git a/tests/testthat/test_OnDiskMSnExp2.R b/tests/testthat/test_OnDiskMSnExp2.R
@@ -141,5 +141,19 @@ test_that("Spectrum processing", {
 })
 
 test_that("removeReporters,OnDiskMSnExp", {
-    ## Hm, lacking test data to compare with.
+    in_mem <- readMSData(f, msLevel. = 2)
+    in_mem_rem <- removeReporters(in_mem, TMT6)
+
+    on_disk <- readMSData2(f)
+    on_disk_2 <- filterMsLevel(on_disk, msLevel. = 2)
+    on_disk_2_rem <- removeReporters(on_disk_2, TMT6)
+    sp_rem <- spectra(on_disk_2_rem)
+    expect_identical(unname(spectra(in_mem_rem)), unname(sp_rem))
+
+    ## Do the call on the full data set.
+    on_disk_rem <- removeReporters(on_disk, TMT6)
+    sp_rem <- spectra(on_disk_rem)
+    ## Subset to MS2 spectra
+    sp_rem <- sp_rem[unlist(lapply(sp_rem, msLevel)) > 1]
+    expect_identical(unname(spectra(in_mem_rem)), unname(sp_rem))
 })