rformassspectrometry / Spectra

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

Add peak variable support to MsBackendMemory and MsBackendDataFrame #297

Closed jorainer closed 9 months ago

jorainer commented 1 year ago

This PR adds the peak variable functionality mentioned in issues #289 (and #287). In essence the promise of peaksData is to return a 2 dimensional data structure. Can be a matrix or data.frame. If only "mz" and "intensity" are requested MsBackendMemory and MsBackendDataFrame return a list of matrix, otherwise a list of data.frame.

lgatto commented 1 year ago

For now, additional peaks variables are only possible for in-memory (i.e. Memory and DataFrame) backends, right? Additional ones could come (for example SQL-based backends), but not (possibly never) for the mzR backend. This could be mentioned/discussed either in the manual page and/or in the vignette.

jorainer commented 1 year ago

For now, additional peaks variables are only possible for in-memory (i.e. Memory and DataFrame) backends, right?

Actually, no. Since e.g. MsBackendMzR and others inherit from MsBackendDataFrame (or MsBackendMemory) they support also peaks variables. For MsBackendMzR they would obviously not (yet) imported from mzML files directly, but it would be possible to add them later (with a future addPeaksVariable function). There is already some notes on peaks variable support of different backends in the (too large) documentation of Spectra objects. But that could then also be improved in the revamp of the docs (issue #288 )

jorainer commented 1 year ago

Thanks @lgatto for the review. I have now addressed the comments.