search

rformassspectrometry / Spectra

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

Add validator method to MsBackend checking for ordered m/z values #261

Closed jorainer closed 1 year ago

jorainer commented 1 year ago

This is based on discussion (and misunderstandings) in PR #259 and related issue #258 : many functions in Spectra assume the m/z values to be sorted (within each spectrum). Currently there is only a validator method for MsBackendDataFrame checking that (https://github.com/rformassspectrometry/Spectra/blob/master/R/MsBackendDataFrame-functions.R#L55:L63).

I'm wondering if it would not be better to enforce m/z sorting by adding a respective validator method directly to MsBackend?

Maybe adding a new method validOrderedMz to MsBackend which checks if m/z values are ordered and that is called within the validObject for MsBackend? That would allow certain backends to implement their own validator function (e.g. using SQL calls and thus avoiding to load all data into memory)?

A concern is however that this might have a quite negative impact on performance (if it's called for each validObject) since it involves loading of all m/z values...

any thoughts @lgatto @sgibb ?

lgatto commented 1 year ago

Unless we have observed/recorded this to be an issue in real life, I don't think it is worth the negative impact on performance.

Don't we check this when the object is constructed? If not, that's where this could be done, only once.

jorainer commented 1 year ago

Agree.