-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Allow notch filtering of epochs #12403
base: main
Are you sure you want to change the base?
Conversation
…ributing guide link
Moved notch_filter to class FilterMixin in filter.py and updated contributing guide link
Hello! 👋 Thanks for opening your first pull request here! ❤️ We will try to get back to you soon. 🚴 |
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.
Looks like a good start! Can you look at why the CIs fail? They should tell you some stuf about what is wrong. If it doesn't make sense I can give more pointers (but I'm hoping they're maybe clear enough!).
Any new change should come with updated tests that ideally fail on main
but pass on the new PR. In your case that would probably involve writing some code that calls epochs.notch_filter
-- that wouldn't exist on main
(and hence fail) but should pass on this PR. I'd suggest around here in test_epochs.py
somewhere since it's where we test other Epochs filtering methods:
mne-python/mne/tests/test_epochs.py
Line 1044 in 87df00d
assert_allclose(epochs.get_data(), data_filt, atol=1e-17) |
It would be awesome if you computed the PSD before and after and checked for example that you could remove 60 Hz noise -- but even starting out with having a call to epochs.notch_filter(...)
would be a good start!
@@ -5,5 +5,5 @@ MNE-Python is maintained by a community of scientists and research labs. The pro | |||
|
|||
Users and contributors to MNE-Python are expected to follow our [code of conduct](https://github.com/mne-tools/.github/blob/main/CODE_OF_CONDUCT.md). | |||
|
|||
The [contributing guide](https://mne.tools/dev/development/contributing.html) has details on the preferred contribution workflow | |||
The [contributing guide](https://mne.tools/stable/development/contributing.html) has details on the preferred contribution workflow |
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.
This one should actually still point to the dev
page because it stays most up-to-date (updated roughly daily), and so is the best guide for current practice
The [contributing guide](https://mne.tools/stable/development/contributing.html) has details on the preferred contribution workflow | |
The [contributing guide](https://mne.tools/dev/development/contributing.html) has details on the preferred contribution workflow |
@ikarabinas are you still interested in working on this? |
I adjusted my workflow and do not need this feature anymore. If this changes in the future, I may come back to this issue then. Thanks! |
Reference issue
Resolves #12398
What does this implement/fix?
Moved notch_filter from mne/io/base.py to class FilterMixin in mne/filter.py to allow filtering of Epoch objects, not only Raw. Fixed broken link to the contributing guidelines.