Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

xcmsSet interface gives errors #700

Open
rickhelmus opened this issue Nov 23, 2023 · 13 comments
Open

xcmsSet interface gives errors #700

rickhelmus opened this issue Nov 23, 2023 · 13 comments

Comments

@rickhelmus
Copy link
Contributor

Hi all,

Recently, CI started to fail where patRoon uses the old xcmsSet interface. I can indeed reproduce this quite easily:

xs <- xcms::xcmsSet(list.files(system.file("cdf", package = "faahKO"), full.names = TRUE, recursive = TRUE)[1],
                    "sample", method = "centWave")

Error:

Error in xcms::xcmsSet(list.files(system.file("cdf", package = "faahKO"),  : 
  Chromatographic peak detection failed for all files! The first error was: Error in validObject(.Object): invalid class “xcmsPeaks” object: superclass "mMatrix" not defined in the environment of the object's class

In case it matters: both CI and the local tests occurred on Windows/R4.2.

Thanks,
Rick

@sneumann
Copy link
Owner

Hi,

can't confirm on my setup, and I wouldn't recall any changes in xcms that could've caused this.

> library(xcms)
...
> xs <- xcms::xcmsSet(list.files(system.file("cdf", package = "faahKO"), full.names = TRUE, recursive = TRUE)[1],
+                     "sample", method = "centWave")

Detecting mass traces at 25 ppm ... OK
Detecting chromatographic peaks in 39815 regions of interest ... OK: 2262 found.
 
> sessionInfo()
R version 4.3.1 (2023-06-16)
Platform: x86_64-pc-linux-gnu (64-bit)
Running under: Ubuntu 22.04.3 LTS
[...]
other attached packages:
[1] xcms_4.1.2          MSnbase_2.25.1      ProtGenerics_1.33.1
[4] S4Vectors_0.36.2    mzR_2.37.1          Rcpp_1.0.10
[7] Biobase_2.58.0      BiocGenerics_0.44.0 BiocParallel_1.32.6

So my best guess right now would be that some other packages interfere, and we have a NAMESPACE issue ?
Can you check a different setup (non-Linux) so we can confirm it is not something fundamentally broken in xcms ?

Yours,
Steffen

@jorainer
Copy link
Collaborator

what puzzles me here is the superclass "mMatrix" - as far as I see xcmsPeaks extends matrix, not mMatrix? Also, I'm suspecting some of these things can start happening when the package namespace is not loaded (like with library(xcms) or functions not being imported through the NAMESPACE, but if functions within xcms are called with xcms:: ....

@rickhelmus
Copy link
Contributor Author

Hi both,

Thank you for your responses. The errors I saw were on Windows, but I could now reproduce it as well under Linux. Some googling of the error hinted that the problem is related to a recent Matrix update, which indeed seems to be the case.

These are the steps I now performed:

  1. Install the latest BioConductor release 3.18 Docker
  2. Run the container with bash
  3. login as the rstudio user (su - rstudio)
  4. Run R and install XCMS and faahKO via BiocManager::install()
  5. Running XCMS should not produce any errors now
  6. Close R, go back to the root shell and update all packages. This is necessary to be done as root, as Matrix will not be updated otherwise (or at least not with BiocManager).
  7. Go back to the user shell and re-run R. The error I reported should appear now.

@jorainer
Copy link
Collaborator

Thanks for the update. Indeed, with Matrix version 1.6.3 we get this error. But IMHO that's more a problem with Matrix then with xcms? seems they are overwriting base R objects? As mentioned above, the xcmsPeaks expands matrix - and we're not including/importing anything from the Matrix package.

@jorainer
Copy link
Collaborator

Maybe it will go away in some future update of Matrix?

@rickhelmus
Copy link
Contributor Author

Actually, this problem might fix itself indeed. Based on this comment, I repeated the test and installed the last XCMS version from GitHub after updating Matrix and now it seems fine. Perhaps this problem will go away when the XCMS binaries at BioC are rebuilt with the latest Matrix.

@jorainer
Copy link
Collaborator

jorainer commented Dec 1, 2023

fingers crossed then ;)

@mukhomorr
Copy link

I had the same issue a few days ago. The problem is in newer mMatrix package versions (1.6.2-1.6.4). Downgrading to 1.6.1. fixed the problem.

rickhelmus added a commit to rickhelmus/patRoon that referenced this issue Dec 9, 2023
@ckeeling
Copy link

...I repeated the test and installed the last XCMS version from GitHub after updating Matrix and now it seems fine...

Thanks for this info @rickhelmus -- I just had the same problem and installing xcms from GitHub after installing Matrix again fixed the issue.

@ckeeling
Copy link

ckeeling commented Feb 15, 2024

Hello @sneumann and @jorainer

I'm trying to understand the version numbers for xcms on GitHub versus BioConductor. Currently, if I install from GitHub with githubinstall("xcms"), it is version xcms_3.21.3, but version xcms_4.0.2 if I install from BioConductor (3.18). I am using IPO held by @rietho for xcms parameter optimization for GCMS samples as described in Metabolomics (2023) 19:26. IPO seems to require the xcms_3.21.3 version from GitHub to not get the above Matrix error during the optimizeXcmsSet step. However, the GitHub version xcms_3.21.3 fails when I later use the fromFile() and chromatogram() functions as described in the xcms tutorial. How can I get IPO and xcms to work without resorting to changing versions between my IPO and xcms scripts?

Thanks for your help,
Chris

@jorainer
Copy link
Collaborator

jorainer commented Mar 8, 2024

sorry, I'm a bit confused - I don't know what exactly the githubinstall("xcms") function does? to install xcms from github you would actually either use remotes::install_github("sneumann/xcms") or BiocManager::install("sneumann/xcms"). The current stable version (for Bioconductor 3.18) can be installed with BiocManager::install("xcms").

The current stable xcms version is 4.0.2
The current development version is 4.1.10

no idea how the githubinstall("xcms") would install an old version - maybe from an old branch?

@jorainer
Copy link
Collaborator

@rickhelmus , in issue #733 we were able to fix the Matrix related error with a (forced) re-installation of the Matrix and xcms packages from source:

BiocManager::install(c("Matrix", "xcms"), type = "source", force = TRUE)

Some of these rather obscure looking errors like the one with the Matrix package seem to stem from the pre-build binary packages from Bioconductor. In short, I guess the xcms binary package was build with a old version of Matrix, then there have been some (rather drastic) updates in the Matrix package, but the xcms binary package was not re-built. Thus, when updating the Matrix package, it did not work well with the installed xcms package that based/was built with the older version.

Hope that this eventually fixes the problem?

@ckeeling
Copy link

Sorry @jorainer for not following up on this. I found that IPO was having other issues that prompted me to stop trying to use IPO altogether, and thus I could work with only the one version from BiocManager::install("sneumann/xcms").

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants