Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add findpeaks as alias for findmaxima #43

Open
KronosTheLate opened this issue May 11, 2024 · 4 comments
Open

Add findpeaks as alias for findmaxima #43

KronosTheLate opened this issue May 11, 2024 · 4 comments

Comments

@KronosTheLate
Copy link
Contributor

As previously discussed, some users are likely to find findpeaks to be a more natural name than findmaxima. There are likely two potential reasons:

  • findpeaks aligns better with the package name "Peaks.jl`
  • Other frameworks such as scipy and MATLAB use 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 export const 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).

@halleysfifthinc
Copy link
Owner

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

@KronosTheLate
Copy link
Contributor Author

I think that should be okay. Perhaps the docstring could start with (Alias: findpeaks). That should make the situation clear, I believe.

@halleysfifthinc
Copy link
Owner

I'll add the alias when I get the chance; I have some other PR's cooking that are a higher priority.

@KronosTheLate
Copy link
Contributor Author

Fair enough - thanks for hearing me out ^_^

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants