rformassspectrometry / Spectra

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

Add a reduce method #282

Closed jorainer closed 1 year ago

jorainer commented 1 year ago

Add a method reduce that groups all peaks within each spectrum based on user provided ppm and tolerance values and keep for each group only the peak with the highest intensity. Could be alternatively called prune, but I think reduce captures the meaning of the function better, i.e. it's purpose is to clean/simplify each spectrum removing peaks with very similar m/z.

lgatto commented 1 year ago

I would suggest to call it reduceSpectra() - reduce is too generic.

jorainer commented 1 year ago

Is this our convention in Spectra? I know, for MsCoreUtils @sgibb wanted to have single verbs (bin, group, ...). I am OK to call it reduceSpectra, it's just a more general question to ensure we have some consistency in the function names (will then eventually also apply to deisotope from issue #283 ?)

lgatto commented 1 year ago

I don't think we should aim for generality in Spectra and would prefer reduceSpectra along the lines of longer, more informative names such as filterPrecursorMz(), ...

I think we can have shorter names in lower-level function/packages such as MsCoreUtils and MetaboCoreUtils, as they work on more general/basic data types, and are aimed for developers, who can be expected to know the nature of the data/objects they manipulate.

AharoniLab commented 1 year ago

'reduceSpectra' sounds just sufficiently focused in my view.