From 24e065754d9532bda282f8ac7f193755784b1c75 Mon Sep 17 00:00:00 2001 From: Philippine Louail <127301965+philouail@users.noreply.github.com> Date: Thu, 5 Dec 2024 16:47:35 +0100 Subject: [PATCH] fix jo's comments --- R/MsBackend.R | 19 ++++++++++++------- R/MsBackendDataFrame.R | 8 +++++--- R/MsBackendMemory.R | 9 ++++++--- R/Spectra.R | 17 +++++++++-------- man/MsBackend.Rd | 10 ++++++---- man/combineSpectra.Rd | 10 ++++++---- 6 files changed, 44 insertions(+), 29 deletions(-) diff --git a/R/MsBackend.R b/R/MsBackend.R index 8a72e416..bf70aca5 100644 --- a/R/MsBackend.R +++ b/R/MsBackend.R @@ -188,7 +188,8 @@ #' #' @param y For `cbind2()`: A `data.frame` or `DataFrame` with the #' spectra variables to be added to the backend. Need to be of the same -#' length as the number of spectra in the backend. +#' length as the number of spectra in the backend. The number of rows and +#' their order has to match the number of spectra and their order in x. #' #' @param x Object extending `MsBackend`. #' @@ -318,9 +319,10 @@ #' for *core* spectra variables. #' #' - `cbind2()`: allows to appends multiple spectra variables to the backend at -#' once. It does so *blindly* and is therefore at the risk of the user. For a -#' more controlled way of adding spectra variables, the `joinSpectraData()` -#' should be used. +#' once. The `Spectra` and the values for the new spectra variables have to +#' be in a matching order. Replacing existing spectra variables is not +#' supported through this function. For a more controlled way of adding +#' spectra variables, the `joinSpectraData()` should be used. #' #' - `centroided()`, `centroided<-`: gets or sets the centroiding #' information of the spectra. `centroided()` returns a `logical` @@ -1042,8 +1044,11 @@ setMethod("cbind2", signature = c("MsBackend", "dataframeOrDataFrameOrmatrix"), function(x, y = data.frame(), ...) { if (is(y, "matrix")) y <- as.data.frame(y) + if (any(colnames(x) %in% colnames(y))) + stop("spectra variables in 'y' are already present in 'x' ", + "replacing them is not allowed") if (nrow(y) != length(x)) - stop("Length of 'y' does not match the number of spectra in 'x'") + stop("Number of row in 'y' does not match the number of spectra in 'x'") for (i in colnames(y)) { x[[i]] <- y[, i] } @@ -1373,7 +1378,7 @@ setMethod("filterRanges", "MsBackend", return(object) if (!is.numeric(ranges)) stop("filterRanges only support filtering for numerical ", - "'spectraVariables'") + "'spectraVariables'") match <- match.arg(match) if (is.character(spectraVariables)){ if(!all(spectraVariables %in% spectraVariables(object))) @@ -1383,7 +1388,7 @@ setMethod("filterRanges", "MsBackend", "function to list possible values.") } else stop("The 'spectraVariables' parameter needs to be of type ", - "'character'.") + "'character'.") if (length(spectraVariables) != length(ranges) / 2) stop("Length of 'ranges' needs to be twice the length of ", "the parameter 'spectraVariables' and define the lower ", diff --git a/R/MsBackendDataFrame.R b/R/MsBackendDataFrame.R index 1fe4f872..7046f103 100644 --- a/R/MsBackendDataFrame.R +++ b/R/MsBackendDataFrame.R @@ -567,8 +567,6 @@ setMethod("[", "MsBackendDataFrame", function(x, i, j, ..., drop = FALSE) { .subset_backend_data_frame(x, i) }) -setClassUnion("dataframeOrDataFrameOrmatrix", - c("data.frame", "DataFrame", "matrix")) #' @importMethodsFrom methods cbind2 #' #' @rdname hidden_aliases @@ -577,8 +575,12 @@ setMethod("cbind2", signature = c("MsBackendDataFrame", function(x, y = data.frame(), ...) { if (is(y, "matrix")) y <- as.data.frame(y) + if (any(colnames(x) %in% colnames(y))) + stop("spectra variables in 'y' are already present in 'x' ", + "replacing them is not allowed") if (nrow(y) != length(x)) - stop("Length of 'y' does not match the number of spectra in 'x'") + stop("Number of row in 'y' does not match the number of ", + "spectra in 'x'") x@spectraData <- cbind(x@spectraData, y) validObject(x) x diff --git a/R/MsBackendMemory.R b/R/MsBackendMemory.R index 8c2b31c4..b25c969d 100644 --- a/R/MsBackendMemory.R +++ b/R/MsBackendMemory.R @@ -670,8 +670,6 @@ setMethod("[", "MsBackendMemory", function(x, i, j, ..., drop = FALSE) { .df_subset(x, i) }) -setClassUnion("dataframeOrDataFrameOrmatrix", - c("data.frame", "DataFrame", "matrix")) #' @importMethodsFrom methods cbind2 #' #' @rdname hidden_aliases @@ -680,8 +678,13 @@ setMethod("cbind2", signature = c("MsBackendMemory", function(x, y = data.frame(), ...) { if (is(y, "matrix")) y <- as.data.frame(y) + if (any(colnames(x) %in% colnames(y))) + stop("spectra variables in 'y' are already present in 'x' ", + "replacing them is not allowed") + if (nrow(y) != length(x)) - stop("Length of 'y' does not match the number of spectra in 'x'") + stop("Number of row in'y' does not match the number of ", + "spectra in 'x'") x@spectraData <- cbind(x@spectraData, y) validObject(x) x diff --git a/R/Spectra.R b/R/Spectra.R index d57c49fa..aa45025a 100644 --- a/R/Spectra.R +++ b/R/Spectra.R @@ -1467,7 +1467,8 @@ setReplaceMethod("[[", "Spectra", function(x, i, j, ..., value) { #' - `cbind2()`: Appends multiple spectra variables from a `data.frame`, #' `DataFrame` or `matrix` to the `Spectra` object at once. It does so #' *blindly* (e.g. do not check rownames compatibility) and is therefore at -#' the risk of the user. For a more controlled way of adding spectra +#' the risk of the user. The function also does not allow to replace existing +#' spectra variables. For a more controlled way of adding spectra #' variables, the `joinSpectraData()` should be used. It will return a #' `Spectra` object with the appended spectra variables. `cbind2()` does #' check however that the number of rows of the `data.frame` or `DataFrame` @@ -1556,8 +1557,9 @@ setReplaceMethod("[[", "Spectra", function(x, i, j, ..., value) { #' @param x A `Spectra` object. #' #' @param y For `joinSpectraData()`: `DataFrame` with the spectra variables -#' to join/add. For `cbind2()`: a `data.frame`, `DataFrame` or -#' `matrix`. +#' to join/add. For `cbind2()`: a `data.frame`, `DataFrame` or +#' `matrix`. The number of rows and their order has to match the +#' number of spectra in `x`, respectively their order. #' #' @param ... Additional arguments. #' @@ -1687,15 +1689,14 @@ setMethod("c", "Spectra", function(x, ...) { .concatenate_spectra(unname(list(unname(x), ...))) }) -setClassUnion("dataframeOrDataFrame", c("data.frame", "DataFrame")) #' @rdname combineSpectra #' #' @export setMethod("cbind2", signature(x = "Spectra", - y = "dataframeOrDataFrame"), function(x, y, ...) { - x@backend <- cbind2(x@backend, y, ...) - x - }) + y = "dataframeOrDataFrameOrmatrix"), + function(x, y, ...) { + x@backend <- cbind2(x@backend, y, ...) + }) #' @rdname combineSpectra setMethod("split", "Spectra", function(x, f, drop = FALSE, ...) { diff --git a/man/MsBackend.Rd b/man/MsBackend.Rd index 833cd2f6..3ff8c4e5 100644 --- a/man/MsBackend.Rd +++ b/man/MsBackend.Rd @@ -332,7 +332,8 @@ backend provides.} \item{y}{For \code{cbind2()}: A \code{data.frame} or \code{DataFrame} with the spectra variables to be added to the backend. Need to be of the same -length as the number of spectra in the backend.} +length as the number of spectra in the backend. The number of rows and +their order has to match the number of spectra and their order in x.} \item{value}{replacement value for \verb{<-} methods. See individual method description or expected data type.} @@ -608,9 +609,10 @@ while columns with only \code{NA}s are removed, a \code{spectraData()} call afte \code{dropNaSpectraVariables()} might still show columns containing \code{NA} values for \emph{core} spectra variables. \item \code{cbind2()}: allows to appends multiple spectra variables to the backend at -once. It does so \emph{blindly} and is therefore at the risk of the user. For a -more controlled way of adding spectra variables, the \code{joinSpectraData()} -should be used. +once. The \code{Spectra} and the values for the new spectra variables have to +be in a matching order. Replacing existing spectra variables is not +supported through this function. For a more controlled way of adding +spectra variables, the \code{joinSpectraData()} should be used. \item \code{centroided()}, \verb{centroided<-}: gets or sets the centroiding information of the spectra. \code{centroided()} returns a \code{logical} vector of length equal to the number of spectra with \code{TRUE} if a diff --git a/man/combineSpectra.Rd b/man/combineSpectra.Rd index 36fedfa1..19fcded3 100644 --- a/man/combineSpectra.Rd +++ b/man/combineSpectra.Rd @@ -7,7 +7,7 @@ \alias{split} \alias{cbind2} \alias{c,Spectra-method} -\alias{cbind2,Spectra,dataframeOrDataFrame-method} +\alias{cbind2,Spectra,dataframeOrDataFrameOrmatrix-method} \alias{split,Spectra,ANY-method} \title{Merging, aggregating and splitting Spectra} \usage{ @@ -26,7 +26,7 @@ joinSpectraData(x, y, by.x = "spectrumId", by.y, suffix.y = ".y") \S4method{c}{Spectra}(x, ...) -\S4method{cbind2}{Spectra,dataframeOrDataFrame}(x, y, ...) +\S4method{cbind2}{Spectra,dataframeOrDataFrameOrmatrix}(x, y, ...) \S4method{split}{Spectra,ANY}(x, f, drop = FALSE, ...) } @@ -54,7 +54,8 @@ of the \linkS4class{MsBackend}.} \item{y}{For \code{joinSpectraData()}: \code{DataFrame} with the spectra variables to join/add. For \code{cbind2()}: a \code{data.frame}, \code{DataFrame} or -\code{matrix}.} +\code{matrix}. The number of rows and their order has to match the +number of spectra in \code{x}, respectively their order.} \item{by.x}{A \code{character(1)} specifying the spectra variable used for merging. Default is \code{"spectrumId"}.} @@ -85,7 +86,8 @@ the \code{\link[=applyProcessing]{applyProcessing()}} function. \item \code{cbind2()}: Appends multiple spectra variables from a \code{data.frame}, \code{DataFrame} or \code{matrix} to the \code{Spectra} object at once. It does so \emph{blindly} (e.g. do not check rownames compatibility) and is therefore at -the risk of the user. For a more controlled way of adding spectra +the risk of the user. The function also does not allow to replace existing +spectra variables. For a more controlled way of adding spectra variables, the \code{joinSpectraData()} should be used. It will return a \code{Spectra} object with the appended spectra variables. \code{cbind2()} does check however that the number of rows of the \code{data.frame} or \code{DataFrame}