- 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