-
Notifications
You must be signed in to change notification settings - Fork 13
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 support for epochs #3
Conversation
meegpowreg/power_features.py
Outdated
@@ -67,8 +67,7 @@ def compute_features( | |||
fbands={'alpha': (8.0, 12.0)}, | |||
clean_func=lambda x: x, | |||
n_jobs=1): | |||
#doc str | |||
|
|||
"""Compute features from raw data or clean epochs.""" |
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.
you can do better. See an example in
https://github.com/mne-tools/mne-python/blob/main/mne/event.py#L24
you could also read this page
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.
@DavidSabbagh @dengemann can you have a look?
covs = list() | ||
for _, fb in fbands.items(): | ||
ec = epochs.copy().load_data().filter(fb[0], fb[1]) | ||
cov = mne.compute_covariance(ec, method='oas', rank=None) |
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.
OK for now to have this hard-coded. Later these parameters should be exposed.
def test_compute_features_epochs(): | ||
raw = mne.io.read_raw_fif(raw_fname, verbose=False) | ||
raw = raw.copy().crop(0, 200).pick( | ||
list(range(2)) + list(range(330, 333)) # take some MEG and EEG |
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.
wouldn't it be more readable to just type the random picks: [0, 1, 330, 331, 332]
?
n_fb = len(fbands) | ||
assert ( | ||
set(features.keys()) == | ||
set(['psds', 'freqs', 'covs', 'xfreqcovs', 'xfreqcorrs', 'cospcovs']) |
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.
you could use set literals here {'psds', 'freqs', ...}
@apmellot thank you for the PR! I have only minor comments. It looks as if this code would allow us to reproduce the pipeline from our papers. If you address the 2 mini issues I have raised OK to merge from my side. |
Thank you for the PR Apolline. I have comments I will share with you tomorrow morning. |
@dengemann you merged this PR but it seems that my comments have not been taken into account. Most of them were cosmetic but my first 2 comments were important I think |
I could not find your comments for some reason. |
@DavidSabbagh you commented but I am sure you forgot to submit the review so we don't see them |
you're right Alex, sorry |
No description provided.