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

[Eric Larson]

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

You can ping previous reviewers to take another look. They can then remove
the tag.

- 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.
>

This unfortunately happens from time to time, especially if it's on the
NumPy dev/master build.

- 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?
>

Yes I think so. Best to raise a sensible error if possible (e.g., argument
shapes don't make sense) but I don't think it's worth spending too much
time protecting against all failure modes.

- 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.
>

Another PR is fine, but we should merge that one before this find_peaks PR
to avoid creating backward compatibility problems.

- 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.
>

I think it makes sense to have this as part of the `find_peaks` PR. Having
good / sufficient documentation of functions is worth holding off on
merging. It is a good test for API sanity, and also makes it easier for
others to review at a high level.

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

There is existing discussion of this in the PR, so if people have thoughts
on this, it's worth looking there to see what ground has been covered first.

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

Thanks for working on this functionality!

Cheers,
Eric
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.python.org/pipermail/scipy-dev/attachments/20180126/4ff8a0e5/attachment-0001.html>