From dc02d602a750d7934c1c0094ec8ba0c8e5dff11b Mon Sep 17 00:00:00 2001
From: jorainer <johannes.rainer@gmail.com>
Date: Thu, 25 May 2023 16:46:57 +0200
Subject: [PATCH] refactor: $,Spectra correctly returns peak variable

---
 NEWS.md                       |  5 +++++
 R/Spectra.R                   | 28 ++++++++++++++++++----------
 man/Spectra.Rd                | 19 +++++++++++--------
 tests/testthat/test_Spectra.R |  2 ++
 4 files changed, 36 insertions(+), 18 deletions(-)

diff --git a/NEWS.md b/NEWS.md
index da3f781d..0124d539 100644
--- a/NEWS.md
+++ b/NEWS.md
@@ -6,6 +6,11 @@
   variables (in addition to `"mz"` and `"intensity"`) are requested. For
   `columns = c("mz", "intensity")` (the default) a `list` of `matrix` is
   returned.
+- `peaksData,Spectra` returns either a `matrix` or `data.frame` and ensures
+  the peak data is correctly subset based on the lazy evaluation processing
+  queue.
+- `$,Spectra` to access peak variables ensures the lazy evaluation queue is
+  applied prior to extracting the values.
 
 ## Changes in 1.11.2
 
diff --git a/R/Spectra.R b/R/Spectra.R
index aab27416..41364844 100644
--- a/R/Spectra.R
+++ b/R/Spectra.R
@@ -227,12 +227,15 @@ NULL
 #'   spectra, each element a `numeric` vector with the m/z values of
 #'   one spectrum.
 #'
-#' - `peaksData`: gets the *peaks* matrices for all spectra in `object`. The
-#'   function returns a [SimpleList()] of matrices, each `matrix` with columns
-#'   `"mz"` and `"intensity"` with the m/z and intensity values for all peaks of
-#'   a spectrum. Optional parameter `columns` is passed to the backend's
-#'   `peaksData` function to allow selection of specific (or additional) peaks
-#'   variables (columns) that should be extracted (if available). Importantly,
+#' - `peaksData`: gets the *peaks* data for all spectra in `object`. Peaks data
+#'   consist of the m/z and intensity values as well as possible additional
+#'   annotations (variables) of all peaks of each spectrum. The function
+#'   returns a [SimpleList()] of two dimensional arrays (either `matrix` or
+#'   `data.frame`), with each array providing the values for the requested
+#'   *peak variables* (by default `"mz"` and `"intensity"`). Optional parameter
+#'   `columns` is passed to the backend's `peaksData` function to allow
+#'   selection of specific (or additional) peaks variables (columns) that
+#'   should be extracted (if available). Importantly,
 #'   it is **not** guaranteed that each backend supports this parameter (while
 #'   each backend must support extraction of `"mz"` and `"intensity"` columns).
 #'   Parameter `columns` defaults to `c("mz", "intensity")` but any value
@@ -246,8 +249,8 @@ NULL
 #'   the backend. Default peak variables are `"mz"` and `"intensity"` (which
 #'   all backends need to support and provide), but some backends might provide
 #'   additional variables.
-#'   These variables correspond to the column names of the `numeric` `matrix`
-#'   representing the peak data (returned by `peaksData`).
+#'   These variables correspond to the column names of the peak data array
+#'   returned by `peaksData`.
 #'
 #' - `polarity`, `polarity<-`: gets or sets the polarity for each
 #'   spectrum.  `polarity` returns an `integer` vector (length equal
@@ -1901,8 +1904,13 @@ setMethod("$", "Spectra", function(x, name) {
         mz(x)
     else if (name == "intensity")
         intensity(x)
-    else
-        do.call("$", list(x@backend, name))
+    else {
+        if (length(x@processingQueue) && name %in% peaksVariables(x))
+            .peaksapply(x, FUN = function(z, ...) z[, name],
+                        columns = c("mz", "intensity", name))
+        else
+            do.call("$", list(x@backend, name))
+    }
 })
 
 #' @rdname Spectra
diff --git a/man/Spectra.Rd b/man/Spectra.Rd
index 7cee4477..84649062 100644
--- a/man/Spectra.Rd
+++ b/man/Spectra.Rd
@@ -872,12 +872,15 @@ level for each spectrum.
 spectra. Returns a \code{\link[=NumericList]{NumericList()}} or length equal to the number of
 spectra, each element a \code{numeric} vector with the m/z values of
 one spectrum.
-\item \code{peaksData}: gets the \emph{peaks} matrices for all spectra in \code{object}. The
-function returns a \code{\link[=SimpleList]{SimpleList()}} of matrices, each \code{matrix} with columns
-\code{"mz"} and \code{"intensity"} with the m/z and intensity values for all peaks of
-a spectrum. Optional parameter \code{columns} is passed to the backend's
-\code{peaksData} function to allow selection of specific (or additional) peaks
-variables (columns) that should be extracted (if available). Importantly,
+\item \code{peaksData}: gets the \emph{peaks} data for all spectra in \code{object}. Peaks data
+consist of the m/z and intensity values as well as possible additional
+annotations (variables) of all peaks of each spectrum. The function
+returns a \code{\link[=SimpleList]{SimpleList()}} of two dimensional arrays (either \code{matrix} or
+\code{data.frame}), with each array providing the values for the requested
+\emph{peak variables} (by default \code{"mz"} and \code{"intensity"}). Optional parameter
+\code{columns} is passed to the backend's \code{peaksData} function to allow
+selection of specific (or additional) peaks variables (columns) that
+should be extracted (if available). Importantly,
 it is \strong{not} guaranteed that each backend supports this parameter (while
 each backend must support extraction of \code{"mz"} and \code{"intensity"} columns).
 Parameter \code{columns} defaults to \code{c("mz", "intensity")} but any value
@@ -890,8 +893,8 @@ support the parameter \code{columns}.
 the backend. Default peak variables are \code{"mz"} and \code{"intensity"} (which
 all backends need to support and provide), but some backends might provide
 additional variables.
-These variables correspond to the column names of the \code{numeric} \code{matrix}
-representing the peak data (returned by \code{peaksData}).
+These variables correspond to the column names of the peak data array
+returned by \code{peaksData}.
 \item \code{polarity}, \verb{polarity<-}: gets or sets the polarity for each
 spectrum.  \code{polarity} returns an \code{integer} vector (length equal
 to the number of spectra), with \code{0} and \code{1} representing negative
diff --git a/tests/testthat/test_Spectra.R b/tests/testthat/test_Spectra.R
index 8f4055da..35d8e2c0 100644
--- a/tests/testthat/test_Spectra.R
+++ b/tests/testthat/test_Spectra.R
@@ -526,6 +526,8 @@ test_that("peaksData,Spectra works", {
     expect_true(is.data.frame(res[[1L]]))
     expect_equal(res[[1L]][, "ann"], c("b", "c", "d"))
     expect_equal(colnames(res[[1L]]), c("ann", "mz"))
+
+    expect_equal(s$ann[[1L]], c("b", "c", "d"))
 })
 
 test_that("lengths,Spectra works", {