[SciPy-Dev] ENH: Extend peak finding capabilities in scipy.signal (#8264)

Lars G. lagru at mailbox.org
Fri Jan 26 14:49:31 EST 2018


Dear SciPy devs,

I'd like to highlight my current pull request which extends SciPy's peak
finding capabilities in the signal module.
https://github.com/scipy/scipy/pull/8264
I've tried to address all feedback that was given (until now) in said
PR. However there are still some areas that would profit from a code
review and an additional pair of eyes.

Furthermore there are still some outstanding issues. It may be best to
discuss them here:

- There are still "Changes requested" which I have addressed or are
outdated. I don't know how to go about removing this flag.

- I think the Travis CI build fail is unrelated to my PR (something in
the stats module). Perhaps someone with more knowledge should confirm this.

- How to handle if invalid peaks are passed to the functions
peak_prominences and peak_widths? Is it okay to describe this as
undefined behavior in the documentation?

- argrelmax doesn't catch peaks that are wider than one sample. Decide
how to deal with this. I have implemented an alternative here which may
even be faster. But I plan to make a performance comparison to confirm
this. I have a feeling this should be addressed (if wanted) in a new
pull request.

- Should the new functionality be covered in the tutorial for
scipy.signal? I feel like that would be the best place to show more
complex examples which are out of place in the docstrings itself. If so,
I think a new PR would be the best place.

- The decision whether find_peaks should have a postfix like
"find_peaks_cwt" is still pending. If so which one?

Again, thank you all in advance for taking the time to review this PR
and any feedback given.

Best regards,
Lars



More information about the SciPy-Dev mailing list