Bug fix: groupval() failing on object converted from XCMSnExp. Issue 471

[andzajan]

Hi, this is proposed fix for #471.

Lines 450 - 453 (https://github.com/andzajan/xcms/blob/master/R/methods-xcmsSet.R#L450-L453) are needed in case if 'phenoData' information is lost during xset <- as(xset, "xcmsSet").

Ping @RJMW, @Tomnl.

[jorainer]

Thanks for the PR @andzajan! Before merging I would however like to wait for the fix proposed by @wkumler (see issue #464) which seems to be upstream of this error. Maybe with that fix we don't need this PR.

[jorainer]

Hi @andzajan , the fix is now in the current master branch (and the RELEASE_3_11 branch). Could you please check if this fixes also your problem?

I guess you know how to install, in case you don't:

## in case you're on Windows paste also the first line:
Sys.setenv(R_REMOTES_NO_ERRORS_FROM_WARNINGS="true")
devtools::install_github("sneumann/xcms")

[andzajan]

Hi @jorainer, unfotunately it's still broken. If I run a test I made in this PR, the groupval now returns empty matrix for converted object.

It's probably small fix, do you want me to have a look into it?

library(xcms)
library(testthat)

cdfs <- dir(system.file("cdf", package = "faahKO"), full.names = TRUE,
recursive = TRUE)[1:3]
xset <- xcmsSet(cdfs, method = 'centWave', ppm = 25, peakwidth = c(20, 80),
    snthresh = 10, prefilter = c(3,100), integrate = 1, mzdiff = -0.001,
    verbose.columns = FALSE, fitgauss = FALSE, noise = 5000)

xset <- xcms::group(xset, method = "density", bw = 30, minfrac = 0.5,
    minsamp = 1)

raw_data <- readMSData(files = cdfs,
    pdata = new("NAnnotatedDataFrame"), mode = "onDisk")
cwp <- xcms::CentWaveParam(peakwidth = c(20, 80), noise = 5000)
xdata <- xcms::findChromPeaks(raw_data, param = cwp)
xdata <- xcms::groupChromPeaks(xdata,
xcms::PeakDensityParam(sampleGroups = rep("KO", 3)))

expect_equivalent(xcms::peaks(xset), xcms::chromPeaks(xdata))
expect_equivalent(xcms::groups(xset),
    as.matrix(xcms::featureDefinitions(xdata)[, 1:8]))

expect_true(is(xdata, "XCMSnExp"))

# Convert XCMSnExp to xcmsSet
xdata <- as(xdata, "xcmsSet")
expect_true(is(xdata, "xcmsSet"))

expect_equivalent(xcms::peaks(xset), xcms::peaks(xdata))
expect_equivalent(xcms::groups(xset), xcms::groups(xdata)[, 1:8])

expect_equivalent(groupval(xset), groupval(xdata))

> head(groupval(xset))
           ko15 ko16 ko18
205/2788     68  629 1362
206/2785     48  614 1342
207.1/2713   30  599 1327
233/3025    107   NA 1398
233.1/3016  101   NA 1394
240.2/3677  423   NA 1628
> head(groupval(xdata))
          
205/2788  
206/2785  
207.1/2713
233/3025  
233.1/3016
240.2/3677

[andzajan]

And it's related to the xdata@phenodata being empty after conversion, that was not behaviour xcms had before Bioconductor 3.11 release.

> sampnames(xdata)
character(0)

There probably is neater fix than this: https://github.com/andzajan/xcms/blob/master/R/methods-xcmsSet.R#L449-L453

sampnum <- seq(length = length(sampnames(object)))
if (length(sampnum) == 0) {
     sampnum <- unique(object@peaks[, "sample"])
}

[jorainer]

Thanks for the suggested fix. I did now introduce a fix upstream, directly in the function to convert the XCMSnExp into xcmsSet see here which should fix this (and potentially also other) problem.

Could you please confirm that this works for you?

You would have to install the jomaster branch as I would like to let the unit test finish before merging.

devtools::install_github("sneumann/xcms", ref = "jomaster")

[andzajan]

Hi @jorainer, I can confirm that all is working as expected now. Thank you for a fast response!

The only difference is that converted xcmsSet object now has file extensions in phenodata slot, while standard object doesn't.

> xdata@phenoData
         file_name
ko15.CDF  ko15.CDF
ko16.CDF  ko16.CDF
ko18.CDF  ko18.CDF

> xset@phenoData
     class
ko15    KO
ko16    KO
ko18    KO

[jorainer]

If you're OK with it, could you please close the pull request @andzajan ?