search

sneumann / xcms

This is the git repository matching the Bioconductor package xcms: LC/MS and GC/MS Data Analysis
Other
177 stars 81 forks source link

[DOC] Replace links to MassSpecWavelet::sav.gol() with links to peakDetectionCWT() #620

Closed zeehio closed 2 years ago

zeehio commented 2 years ago

Hi,

I recently started maintaining the MassSpecWavelet package, a dependency of xcms.

My roadmap is to maintain MassSpecWavelet making zero changes, or very minimal if I have to. I believe it is a rather old and stable package and I like it that way.

However, I have found a bug in the Savitzky-Golay filter implemented there, that affects the boundaries of the signal. It's not a serious issue, since given a filter of length N it only affects the N/2 first and last points of the signal, but still I wanted to correct it.

The MassSpecWavelet:::sav.gol() function was not exported, so it was easy to make all the necessary changes without concerns of breaking other packages. With respect to code I used a Suggests: signal and I used the signal::sgolayfilt() function instead, which has been stable for the last decade at least as well.

With respect to the documentation, sav.gol() had a man/ page and xcms is referencing it. I did not want to break your man pages, so I just created an alias that redirects sav.gol to the peakDetectionCWT, which is the relevant function that calls the Savitzky-Golay filter.

These changes are all released in MassSpecWavelet in the current (3.15) Bioconductor release.

This pull request replaces references to the sav.gol page from the XCMS package. I would appreciate if you merge it so I can remove the sav.gol alias from MassSpecWavelet in a future version, effectively removing all references to sav.gol.

See https://github.com/zeehio/MassSpecWavelet/issues/2 for further details on this issue.

Thanks for your time!

jorainer commented 2 years ago

Thanks @zeehio !