Add new plotPrecursorIons function and address issue #742

sneumann / xcms

This is the git repository matching the Bioconductor package xcms: LC/MS and GC/MS Data Analysis

Other

177 stars 81 forks source link

Add new plotPrecursorIons function and address issue #742 #743

Closed jorainer closed 4 weeks ago

jorainer commented 2 months ago

In this PR:

add new function plotPrecursorIons to visualize location of precursor ions in m/z - retention time plane.
support exclusion of samples/groups from the definition of new features using the PeakDensity correspondence method (issue #742 ).

Unit tests and documentation included.

jorainer commented 2 months ago

@sneumann , I leave it up to you whether we want this small updates to be included in the upcoming release version or not...

philouail commented 2 months ago

Sorry @jorainer I just tested locally and realized something is wrong.

Can you also update the functions-Params.R document (line 239) to allow for NAs in sampleGroups ?

jorainer commented 2 months ago

good catch @philouail ! I've changed it now.

jorainer commented 2 months ago

Changed to draft because there seem to be some problems...

jorainer commented 1 month ago

OK, now we have also a fix for a bug that Phili spotted when testing the code (although the bug was not related to the present code).

jorainer commented 1 month ago

@sneumann , can you please have a look at this PR and eventually merge if you're OK?

sneumann commented 4 weeks ago

Looks good, thanks! I haven't tested if there is any fallout if the PeakDensityParam() handling NAs in other downstream functions, like peak filling or plotting, is there any "worst-case" this could lead to unexpected/degenerate feature groups violating assumptions we take for granted ? Yours, Steffen

jorainer commented 3 weeks ago

There shouldn't be any issue - no other function/method uses this parameter - and the PeakDensityParam() handles it well - so, should be all good, thanks.