search

lgatto / MSnbase

Base Classes and Functions for Mass Spectrometry and Proteomics
http://lgatto.github.io/MSnbase/
126 stars 46 forks source link

Possible bug in descendPeaks #583

Closed Pascallio closed 1 year ago

Pascallio commented 2 years ago

Dear developers,

I might have encountered a bug for an unlikely, but possible situation in MSnbase:::descendPeaks. According to the documentation, "the peak region is defined by descending from the identified centroid/peak on both sides until the measured signal increases again.". However, in situations where the centroid is either the first or last signal in a spectrum, the other side of the peak seems to be ignored.

Some example code for reproducibility:

mz <- c(100.14, 100.145, 100.15) # masses
intensity <- c(1000, 700, 600) # intensities
peakIdx <- 1 # position of the centroid

MSnbase:::descendPeak(mz = mz, intensity = intensity, peakIdx = peakIdx)

This results in a mass of 100.14 and does not take other values than the centroid into account (which in this example would have resulted in 100.1441). It seems that MSnbase:::descendPeak calculates the half_width, which in this case results in 0 and therefore only considers the centroid itself.

Is this the correct behaviour or should the remaining signals be taken into account until the (first) observed signal rises again?

Best, Pascal

jorainer commented 2 years ago

Dear Pascal! Thanks for the pointing this out! Would you be willing to provide a pull request that fixes this issue?

Pascallio commented 2 years ago

Hi Johannes,

Yes ofcourse, I've just written a fix and provided the pull request.

Cheers, Pascal

lgatto commented 1 year ago

I have merged and pushed to Bioc in devel and release. Thank you very much @Pascallio !