-
Notifications
You must be signed in to change notification settings - Fork 191
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 causal filtering to filter.py #3172
Add causal filtering to filter.py #3172
Conversation
for more information, see https://pre-commit.ci
Co-authored-by: Alessio Buccino <[email protected]>
Co-authored-by: Alessio Buccino <[email protected]>
Co-authored-by: Alessio Buccino <[email protected]>
Co-authored-by: Alessio Buccino <[email protected]>
Co-authored-by: Alessio Buccino <[email protected]>
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
Co-authored-by: Alessio Buccino <[email protected]>
for more information, see https://pre-commit.ci
Co-authored-by: Alessio Buccino <[email protected]>
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
Also adding in fixes #2943 in your description will link your issue and automatically close it when merged (for future reference) :) |
for more information, see https://pre-commit.ci
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @JuanPimientoCaicedo thanks a lot for this, it's looking great and is really useful. I'm looking forward to hearing more about these switched capacitor filters and seeing how the phase shifts can be accounted for with this new functionality.
Just made a few small suggestions around docstrings, I just pushed some additional tests let me know what you think, sorry I should have warned before pushing!
…icedo/spikeinterface into add-causal-filter
Co-authored-by: Joe Ziminski <[email protected]>
for more information, see https://pre-commit.ci
Thank you for taking a look, @JoeZiminski. You are free to push commits, you know the library better than me. I added the documentation changes you suggested! |
@alejoe91 @JoeZiminski. Hi guys, do you think this is mature enough to merge? Cheers. |
LGTM! |
@JuanPimientoCaicedo LGTM! Thanks a lot for this, its a really cool addition, and I'm looking forward to trying it out once more details on the headstage frequency response is known! |
Hi, guys. I am closing #3133.