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

🐍 Rename spike detector -> threshold detector #1976

Merged

Conversation

thorstenhater
Copy link
Contributor

Rename spike_detector to threshold_detector and make spike_detector throw a deprecation error.
This brings the Python API more in line with C++ and is closer to the object's actual function.

Closes #1642

Copy link
Contributor

@schmitts schmitts left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We just break API? I'm fine with that.

@thorstenhater
Copy link
Contributor Author

Yes, of course. But: less confusion across C++/Python and a clearer name. It's also still pre-1.0

Copy link
Contributor

@brenthuisman brenthuisman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes!!!!!!

@thorstenhater
Copy link
Contributor Author

I appreciate the excitement for homogenising APIs, but any particular reason?

@brenthuisman
Copy link
Contributor

This name has been confusing me for some time now.

@thorstenhater thorstenhater merged commit d0b31a6 into arbor-sim:master Sep 16, 2022
@thorstenhater thorstenhater deleted the ux/spike-detector-naming branch September 16, 2022 07:26
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

Successfully merging this pull request may close these issues.

Rename spike_detector to something more descriptive
3 participants