Merge pull request #23 from rformassspectrometry/phili

thanks for the review, I fixed the comments. I'm merging !

.github/workflows/check-bioc.yml

@@ -207,7 +207,7 @@ jobs:
           rcmdcheck::rcmdcheck(
               args = c("--no-build-vignettes", "--no-manual", "--timings"),
               build_args = c("--no-manual", "--no-resave-data"),
-              error_on = "warning",
+              error_on = "error",
               check_dir = "check"
           )
         shell: Rscript {0}

DESCRIPTION

@@ -13,11 +13,11 @@ Authors@R: c(person(given = "Laurent", family = "Gatto",
 		    role = c("aut")),
 	     person(given = "Johannes", family = "Rainer",
 		    email = "Johannes.Rainer@eurac.edu",
-		    role = c("aut", "cre"),
+		    role = c("aut"),
 		    comment = c(ORCID = "0000-0002-6977-7147")),
 	     person(given = "Philippine", family = "Louail",
 		    email = "philippine.louail@eurac.edu",
-		    role = c("aut"),
+		    role = c("aut", "cre"),
 		    comment = c(ORCID = "0009-0007-5429-6846")))
 Depends:
     ProtGenerics (>= 1.35.4)
@@ -32,10 +32,15 @@ Suggests:
     rmarkdown,
     BiocStyle
 License: Artistic-2.0
+Encoding: UTF-8
 LazyData: yes
 VignetteBuilder: knitr
 BugReports: https://github.com/RforMassSpectrometry/Chromatograms/issues
 URL: https://github.com/RforMassSpectrometry/Chromatograms
 biocViews: Infrastructure, Proteomics, MassSpectrometry, Metabolomics
 Roxygen: list(markdown=TRUE)
-RoxygenNote: 7.3.1
+RoxygenNote: 7.3.2
+Collate: 
+    'AllGenerics.R'
+    'ChromBackend-functions.R'
+    'ChromBackend.R'

NAMESPACE

@@ -1,13 +1,16 @@
 # Generated by roxygen2: do not edit by hand
 
 export(coreChromVariables)
+export(corePeaksVariables)
 export(fillCoreChromVariables)
 export(validChromData)
+export(validPeaksData)
 exportClasses(ChromBackend)
 exportMethods("$")
 exportMethods("$<-")
 exportMethods("[")
 exportMethods("chromData<-")
+exportMethods("chromIndex<-")
 exportMethods("collisionEnergy<-")
 exportMethods("dataOrigin<-")
 exportMethods("dataStorage<-")
@@ -33,11 +36,6 @@ exportMethods(chromVariables)
 exportMethods(collisionEnergy)
 exportMethods(dataOrigin)
 exportMethods(dataStorage)
-exportMethods(filterDataOrigin)
-exportMethods(filterDataStorage)
-exportMethods(filterMsLevel)
-exportMethods(filterMzRange)
-exportMethods(filterMzValues)
 exportMethods(intensity)
 exportMethods(isEmpty)
 exportMethods(isReadOnly)
@@ -54,16 +52,14 @@ exportMethods(precursorMzMin)
 exportMethods(productMz)
 exportMethods(productMzMax)
 exportMethods(productMzMin)
+exportMethods(reset)
 exportMethods(rtime)
 exportMethods(selectChromVariables)
 exportMethods(split)
 importFrom(IRanges,NumericList)
-importFrom(MsCoreUtils,between)
-importFrom(MsCoreUtils,common)
 importFrom(methods,.valueClassTest)
 importFrom(methods,as)
 importFrom(methods,is)
-importFrom(methods,new)
 importFrom(methods,validObject)
 importMethodsFrom(ProtGenerics,"collisionEnergy<-")
 importMethodsFrom(ProtGenerics,"dataOrigin<-")
@@ -81,11 +77,6 @@ importMethodsFrom(ProtGenerics,backendParallelFactor)
 importMethodsFrom(ProtGenerics,collisionEnergy)
 importMethodsFrom(ProtGenerics,dataOrigin)
 importMethodsFrom(ProtGenerics,dataStorage)
-importMethodsFrom(ProtGenerics,filterDataOrigin)
-importMethodsFrom(ProtGenerics,filterDataStorage)
-importMethodsFrom(ProtGenerics,filterMsLevel)
-importMethodsFrom(ProtGenerics,filterMzRange)
-importMethodsFrom(ProtGenerics,filterMzValues)
 importMethodsFrom(ProtGenerics,intensity)
 importMethodsFrom(ProtGenerics,isReadOnly)
 importMethodsFrom(ProtGenerics,msLevel)

R/AllGenerics.R

@@ -6,6 +6,9 @@ setGeneric("chromData<-", function(object, value)
 #' @rdname ChromBackend
 setGeneric("chromIndex", function(object, ...) standardGeneric("chromIndex"))
 #' @rdname ChromBackend
+setGeneric("chromIndex<-", function(object, value)
+    standardGeneric("chromIndex<-"))
+#' @rdname ChromBackend
 setGeneric("chromNames", function(object, ...) standardGeneric("chromNames"))
 #' @rdname ChromBackend
 setGeneric("chromNames<-", function(object, value)
@@ -48,3 +51,6 @@ setGeneric("productMzMin<-", function(object, value)
 #' @rdname ChromBackend
 setGeneric("selectChromVariables", function(object, ...)
     standardGeneric("selectChromVariables"))
+#' @rdname ChromBackend
+setGeneric("reset", function(object, ...)
+    standardGeneric("reset"))

R/ChromBackend-functions.R

@@ -1,43 +1,17 @@
-.valid_chrom_backend_data_storage <- function(x) {
-    if (anyNA(x))
-        return("'NA' values in dataStorage are not allowed.")
-    NULL
-}
-
-#' Helper function that matches `x` against `mz` (using the `closest` function)
-#' and returns the indices of `x` that match any of the values in `mz`. The
-#' function takes care of sorting `x` and `mz` and deals also with missing
-#' values.
-#'
-#' @return `integer` with the indices of values in `x` that are not `NA` and
-#'     are matching any of the values in `mz` given `ppm` and `tolerance`.
-#'
-#' @noRd
-#'
-#' @importFrom MsCoreUtils common
-#'
-#' @author Sebastian Gibb, Johannes Rainer
-.values_match_mz <- function(x, mz, ppm = 20, tolerance = 0) {
-    o <- order(x, na.last = NA)
-    cmn <- common(x[o], sort(mz), tolerance = tolerance, ppm = ppm,
-                  duplicates = "keep", .check = FALSE)
-    sort(o[cmn])
-}
-
 #' @rdname ChromBackend
 #'
 #' @export
 coreChromVariables <- function() .CORE_CHROM_VARIABLES
 
-#' *core* chromatogram variables with expected data type:
+#' *core* chromatogram variables with expected data type: `integer`, `numeric`,
+#' and `character`. Must be a single value.
 #'
 #' @noRd
 .CORE_CHROM_VARIABLES <- c(
     chromIndex = "integer",
     collisionEnergy = "numeric",
     dataOrigin = "character",
     dataStorage = "character",
-    intensity = "NumericList",
     msLevel = "integer",
     mz = "numeric",
     mzMin = "numeric",
@@ -47,19 +21,17 @@ coreChromVariables <- function() .CORE_CHROM_VARIABLES
     precursorMzMax = "numeric",
     productMz = "numeric",
     productMzMin = "numeric",
-    productMzMax = "numeric",
-    rtime = "NumericList"
+    productMzMax = "numeric"
     )
 
 #' @title Fill data frame with columns for missing core chrom variables
 #'
 #' @description
 #'
-#' `fillCoreChromVariables()` fills a provided `data.frame` (or `DataFrame`)
-#' with columns for eventually missing *core* chromatogram variables **except**
-#' peaks variables (i.e. `"intensity"` and `"rtime"`). The missing core
-#' variables are added as new columns with missing values (`NA`) of the
-#' correct data type.
+#' `fillCoreChromVariables()` fills a provided `data.frame`
+#' with columns for eventually missing *core* chromatogram variables.
+#' The missing core variables are added as new columns with missing values
+#' (`NA`) of the correct data type.
 #' Use [coreChromVariables()] to list the set of core variables and their data
 #' types.
 #'
@@ -85,10 +57,11 @@ coreChromVariables <- function() .CORE_CHROM_VARIABLES
 #' ## missing values).
 fillCoreChromVariables <- function(x = data.frame()) {
     nr <- nrow(x)
-    cv <- .CORE_CHROM_VARIABLES[!names(.CORE_CHROM_VARIABLES) %in%
-                                c("intensity", "rtime")]
-    miss <- setdiff(names(cv), colnames(x))
-    cbind(x, lapply(cv, function(z, n) rep(as(NA, z), n), nr))
+    cv <- .CORE_CHROM_VARIABLES
+    miss <- cv[setdiff(names(cv), colnames(x))]
+    if (!length(miss))
+        return(x)
+    cbind(x, lapply(miss, function(z, n) rep(as(NA, z), n), nr))
 }
 
 #' @title Check core chromatogram variables for correct data types
@@ -111,8 +84,11 @@ fillCoreChromVariables <- function(x = data.frame()) {
 #' `character` specifying which variables/columns don't have the correct
 #' type (for `error = FALSE`).
 #'
+#' @importFrom methods is
+#'
 #' @export
 validChromData <- function(x = data.frame(), error = TRUE) {
+    .valid_chrom_backend_data_storage(x$dataStorage)
     cn <- intersect(colnames(x), names(.CORE_CHROM_VARIABLES))
     msg <- unlist(lapply(cn, function(z) {
         if (!is(x[, z], .CORE_CHROM_VARIABLES[z]))
@@ -124,17 +100,48 @@ validChromData <- function(x = data.frame(), error = TRUE) {
     else msg
 }
 
-#' Create a list of empty peak matrices
+#' *core* peaks variables with expected data type:`numeric`, and
+#' must be plural.
 #'
-#' @param x `integer` with the number of (empty) matrices to create.
+#' @noRd
+.CORE_PEAKS_VARIABLES <- c(
+    intensity = "numeric",
+    rtime = "numeric"
+)
+
+#' @rdname ChromBackend
 #'
-#' @param columns `character` with the column names for each peak matrix
+#' @export
+corePeaksVariables <- function() .CORE_PEAKS_VARIABLES
+
+#' `validPeaksData()` checks that the names of the input peaksData list,
+#' representing *core* peaks variables are of the correct data type.
 #'
-#' @return `list` of length `x` with 0-row peak matrices
+#' @param x `list` representing the peaks data of a `Chromatograms`
 #'
-#' @noRd
-.empty_peaks_data <- function(x, columns = c("mz", "intensity")) {
-    emat <- matrix(numeric(), ncol = length(columns), nrow = 0,
-                   dimnames = list(character(), columns))
-    replicate(x, emat, simplify = FALSE)
+#' @importFrom methods is
+#'
+#' @export
+
+#' @rdname hidden_aliases
+validPeaksData <- function(x = list()) {
+    if (!is.list(x)) {
+        stop("'peaksData' must be a 'list'")
+    }
+    lapply(x, function(df) {
+        if (!is.data.frame(df))
+            stop("All the peaksData entries should be of class 'data.frame'")
+        pv <- intersect(colnames(df), names(.CORE_PEAKS_VARIABLES))
+        if (!all(vapply(pv, function(col) is(df[[col]]
+                                             , .CORE_PEAKS_VARIABLES[[col]]),
+                        logical(1))))
+            stop("The peaksData variable ", col, " has the wrong data type.")
+    })
 }
+
+#' @noRd
+.valid_chrom_backend_data_storage <- function(x) {
+    if (anyNA(x))
+        return("'NA' values in dataStorage are not allowed.")
+    NULL
+}  ## what's this ?