Open KronosTheLate opened 4 months ago
That would be fine. If we just exported findpeaks
as an alias for findmaxima
, ?findpeaks
pulls up the findmaxima
docstring. Do you think that would be confusing, meh, or a good/helpful indicator that there are different kinds of peaks (e.g. minima)?
I think that should be okay. Perhaps the docstring could start with (Alias: findpeaks
). That should make the situation clear, I believe.
I'll add the alias when I get the chance; I have some other PR's cooking that are a higher priority.
Fair enough - thanks for hearing me out ^_^
As previously discussed, some users are likely to find
findpeaks
to be a more natural name thanfindmaxima
. There are likely two potential reasons:findpeaks
aligns better with the package name "Peaks.jl`findpeaks
for this functionality.In my opinion, the second point weighs the most. But the first also makes obvious to me that this is a good idea.
Because
findmaxima
is already exported, and provides a more explicit distinction between minima (findminima
) and maxima, it is useful to keep around. But it would not hurt to exportconst findpeaks = findmaxima
(I think const is good here...?), which would not increase the "surface area" of the source code (no extra docstring, the underlying function is the only function to compile, etc).