Weighted moving average smoothing of spectra #273
Conversation
There was a problem hiding this comment.
Thanks for your PR. This won't be merged before sgibb/MALDIquant#54 isn't merged and MALDIqant 1.17 is available on CRAN.
Please also add MALDIquant >= 1.17 to the DESCRIPTION file.
R/functions-Spectrum.R
Outdated
| @@ -439,7 +439,7 @@ pickPeaks_Spectrum <- function(object, halfWindowSize = 2L, | |||
| } | |||
|
|
|||
| smooth_Spectrum <- function(object, | |||
| method = c("SavitzkyGolay", "MovingAverage"), | |||
| method = c("SavitzkyGolay", "MovingAverage","MovingWeightedAverage"), | |||
There was a problem hiding this comment.
Please add a space after the comma. We try to follow the Bioconductor coding style: https://bioconductor.org/developers/how-to/coding-style/
man/smooth-methods.Rd
Outdated
| @@ -26,7 +27,8 @@ | |||
|
|
|||
| Smooths all spectra in \code{MSnExp}. \code{method} could be | |||
| \code{"SavitzkyGolay"} or | |||
There was a problem hiding this comment.
Please remove the "or" and replace it by a comma, reads: SavitzkyGolay, MovingAverage or MovingWeightedAverage.
|
Thanks for the helpful comments. I'm working on an updated pull also reflecting the suggestions in sgibb/MALDIquant#54. |
|
I guess this PR is not needed anymore. MALDIquant 1.17 introduces a new "weighted" argument to |
|
The only thing that was to do in the end was to change the man page to reflect the changes in MALDIquant. I leave it up to you guys if you want to reopen this one or change the man pages at a later time, perhaps when something more substantial is to be done there? |
As explained in #272