Skip to content

Commit

Permalink
fix: ensure ms_level column in chromPeaksData is integer
Browse files Browse the repository at this point in the history
- Ensure the "ms_level" column in the `chromPeaksData` is of type `integer` to
  avoid obscure validity errors on extracted `Chromatogram` objects (happening
  randomly).
  • Loading branch information
jorainer committed Nov 30, 2023
1 parent 05d762e commit e06f7b8
Show file tree
Hide file tree
Showing 5 changed files with 12 additions and 8 deletions.
11 changes: 4 additions & 7 deletions R/XcmsExperiment.R
Original file line number Diff line number Diff line change
Expand Up @@ -858,7 +858,7 @@ setMethod(
chunkSize = chunkSize, BPPARAM = BPPARAM)
}
## Assign/define peak IDs.
pkd <- data.frame(ms_level = rep(msLevel, nrow(res)),
pkd <- data.frame(ms_level = rep(as.integer(msLevel), nrow(res)),
is_filled = rep(FALSE, nrow(res)))
ph <- XProcessHistory(param = param,
type. = .PROCSTEP.PEAK.DETECTION,
Expand Down Expand Up @@ -1224,7 +1224,7 @@ setMethod(
maxi <- max(
0, as.integer(sub("CP", "", rownames(chromPeaks(object)))))
rownames(res) <- .featureIDs(nr, "CP", maxi + 1)
pkd <- data.frame(ms_level = rep(msLevel, nr),
pkd <- data.frame(ms_level = rep(as.integer(msLevel), nr),
is_filled = rep(FALSE, nr))
rownames(pkd) <- rownames(res)
pb$tick()
Expand Down Expand Up @@ -1554,7 +1554,7 @@ setMethod(
stop("No chromatographic peaks present. ",
"Please run 'findChromPeaks' first.")
res <- .manual_feature_definitions(chromPeaks(object), peakIdx)
res$ms_level <- msLevel
res$ms_level <- as.integer(msLevel)
if (hasFeatures(object)) {
maxi <- max(as.integer(
sub("FT", "", rownames(featureDefinitions(object)))))
Expand Down Expand Up @@ -1840,7 +1840,7 @@ setMethod(
nr <- nrow(res)
maxi <- max(as.integer(sub("CP", "", rownames(chromPeaks(object)))))
rownames(res) <- .featureIDs(nr, "CP", maxi + 1)
cpd <- data.frame(ms_level = rep(msLevel, nr),
cpd <- data.frame(ms_level = rep(as.integer(msLevel), nr),
is_filled = rep(TRUE, nr))
rownames(cpd) <- rownames(res)
object@chromPeaks <- rbind(object@chromPeaks, res)
Expand Down Expand Up @@ -1976,6 +1976,3 @@ setMethod(
object[i = sort(unique(file)), keepAdjustedRtime = keepAdjustedRtime,
keepFeatures = keepFeatures, ...]
})

## TODO filterMsLevel? Function not yet needed. In case, needs also an
## implementation for MsExperiment: update the spectra-sample-mapping.
1 change: 1 addition & 0 deletions R/functions-utils.R
Original file line number Diff line number Diff line change
Expand Up @@ -816,6 +816,7 @@ groupOverlaps <- function(xmin, xmax) {
pks_tmz = rep(1L, nrow(pks)),
aggregationFun = "sum") {
nr <- nrow(pks)
pks_msl <- as.integer(pks_msl)
FUN <- switch(aggregationFun,
"sum" = getFunction("sumi"),
"max" = getFunction("maxi"),
Expand Down
2 changes: 2 additions & 0 deletions inst/NEWS
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
Changes in version 4.1.4
----------------------

- Fix errors related to invalid `Chromatogram` objects extracted from xcms
results: ensure MS level in `chromPeaksMatrix` is `integer`.
- Fix definition of anchor peaks for peakGroups alignment with subset
(issue #702).
- Add `filterMsLevel` method for `MsExperiment` and `XcmsExperiment`.
Expand Down
2 changes: 1 addition & 1 deletion tests/testthat/test_Chromatogram.R
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
## test_that("extractChromatograms is deprecated", {
skip_on_os(os = "windows", arch = "i386")
## skip_on_os(os = "windows", arch = "i386")

## expect_warning(chrs <- extractChromatograms(
## filterRt(filterFile(od_x, file = 2), c(2500, 2600))))
Expand Down
4 changes: 4 additions & 0 deletions tests/testthat/test_XcmsExperiment.R
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ test_that("findChromPeaks,MsExperiment et al works", {
expect_true(is.data.frame(chromPeakData(xmse, return.type = "data.frame")))
expect_s4_class(chromPeakData(xmse), "DataFrame")
expect_true(nrow(chromPeakData(xmse, 2:3)) == 0)
expect_true(is.integer(chromPeakData(res)$ms_level))

## dropChromPeaks
rres <- dropChromPeaks(res)
Expand All @@ -73,13 +74,16 @@ test_that("findChromPeaks,MsExperiment et al works", {
expect_equal(res@chromPeaks, res2@chromPeaks)
expect_equal(res@chromPeakData, res2@chromPeakData)
expect_true(length(res2@processHistory) == 2)
expect_true(is.integer(chromPeakData(res2)$ms_level))

res2 <- findChromPeaks(res, param = p, msLevel = 2L, add = FALSE)
expect_equal(nrow(res2@chromPeaks), 0)
expect_true(length(res2@processHistory) == 1)
expect_true(is.integer(chromPeakData(res2)$ms_level))

res2 <- findChromPeaks(mse, param = p, chunkSize = -1)
expect_equal(res@chromPeaks, res2@chromPeaks)
expect_true(is.integer(chromPeakData(res2)$ms_level))

expect_true(hasChromPeaks(res))
expect_true(hasChromPeaks(res, msLevel = 1L))
Expand Down

0 comments on commit e06f7b8

Please sign in to comment.