rformassspectrometry / Spectra

Low level infrastructure to handle MS spectra
https://rformassspectrometry.github.io/Spectra/
34 stars 24 forks source link

AnnotationFilter-based filtering of Spectra objects? #313

Closed jorainer closed 3 months ago

jorainer commented 4 months ago

Discussion whether it would make sense to support filtering of Spectra with more natural R expressions, as provided/used e.g. in Bioconductor's AnnotationFilter package. Examples would be:

Filter a Spectra to spectra with a retention time between 100 and 200 seconds:

filterSpectra(sps, filter = ~ rtime > 100 & rtime < 200)

Subset to spectra matching (any) provided MS level(s):

filterSpectra(sps, filter = ~ msLevel %in% 2:3)

Get all spectra with their precursor m/z matching (any) of the provided values. The tolerance criteria would need to be passed as additional parameter, e.g. using ppm = 10:

filterSpectra(sps, filter = ~ precursorMz %in% c(123.3, 343.45), ppm = 10)

While it would be relatively straight forward (and easy) to implement this functionality, the main question is whether it really makes sense. To me, this would only make sense if it was more intuitive to the user than the dedicated filter methods (filterRt, filterMsLevel, filterPrecursorMzValues).

Potential advantage:

Disadvantage:

Anyway, as a developer of AnnotationFilter and having used this type of filters in ensembldb I'm a bit biased towards this approach. Would be nice to know what others think of that. @lgatto , @sgibb , @philouail , @michaelwitting @adafede any opinion/thoughts on this?

Adafede commented 4 months ago

Hi! Thank you for tagging me on this one!

I (personnally) do not see it as a large improvement in regard to the intuitiveness ? (So second your thought of it not necessarily prioritary)

Could you give an example of a | logic that would not be feasible easily without this implementation (& is rather straightforward)?

Would you plan also having it for things like containsMz, containsNeutralLoss and consorts?

Maybe I also misunderstood part of the discussion, but isn't also related to SpectraQL?

jorainer commented 4 months ago

thanks @Adafede for your input! that's exactly why I posted this question. I agree, also after discussions with Phili, that this might not make too much sense in the end. It's not as efficient and complete as all the filterXXX functions we have, and yes, at least part of it can be done with SpectraQL. So, thanks again for your feedback!

jorainer commented 3 months ago

Will close issue, as we will not implement this...