sneumann / xcms

This is the git repository matching the Bioconductor package xcms: LC/MS and GC/MS Data Analysis
Other
185 stars 80 forks source link

status of Warpgroup #364

Open tentrillion opened 5 years ago

tentrillion commented 5 years ago

XCMS team, I was recently re-reading the Warpgroup paper from Gary Patti's group. I think this algorithm came out before xcms3. I was wondering if any of these ideas would make sense to integrate into XCMS.

Does xcms3 make the Warpgroup technique superfluous? Looking forward to thoughts on the issue. I'd also be happy to help if it is useful. I'm a pretty experienced R user and modestly experienced developer, for a wet lab person.

Potential labels for this issue: xcms3, enhancement

jorainer commented 5 years ago

We haven't integrated warpgroup into xcms. To me that sounds like an interesting idea and contributions in the form of pull requests are anyway highly welcome! (@sneumann any comments?).

bouranij commented 5 years ago

I am working with samples that contain stable-isotope labels and I find that WarpGroup handles the grouping and detection of peaks much better than CentWave and Obiwarp. The main issue I run into is that WarpGroup (and X13CMS which I also utilize) rely on xcmsSet objects while xcms3 now creates XCMSnExp objects. I am able to convert my XCMSnExp objects down to xcmsSet objects but it comes at the cost of losing my MS2 data. Is there a way around this issue?

sneumann commented 5 years ago

Hi @bouranij, I am afraid not, which was the reason we introduced XCMSnExp :-) maybe you could keep some mapping information between xcmsSet and XCMSnExp ? I could imagine that the authors of those package would appreciate help porting their packages to xcms3. Depending on the packages, porting might be fairly straightforward. Yours, Steffen

sneumann commented 5 years ago

Hi @tentrillion , the abstract of the Patti paper says The package includes example data and XCMS compatibility wrappers for ease of use adn as said above, I'd guess @nathaniel-mahieu wouldn't mind a PR adding a wrapper that takes an xcms3 object. Yours, Steffen