-
Notifications
You must be signed in to change notification settings - Fork 42
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
variables renaming for consistency in prep.py + tests #197
variables renaming for consistency in prep.py + tests #197
Conversation
@@ -347,7 +347,7 @@ def pair_connectivity(data: Union[list, np.ndarray], sampling_rate: int, frequen | |||
elif type(frequencies) == dict: | |||
values = compute_freq_bands(data, sampling_rate, frequencies) | |||
else: | |||
TypeError("Please use a list or a dictionary to specify frequencies.") | |||
raise TypeError("Please use a list or a dictionary to specify frequencies.") |
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.
exceptions must be raised in order to interrupt the execution flow
for raw in raw_S: | ||
raws.append(mne.io.Raw.filter(raw, l_freq=2., h_freq=None)) | ||
|
||
raws = [mne.io.Raw.filter(raw, l_freq=freqs[0], h_freq=freqs[1]) for raw in raw_S] |
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.
use list comprehension in multiple places to take advantage of type inference
" Index begins at zero. (If you do not want to apply" | ||
" ICA on your data, do not enter nothing and press enter.)") | ||
comp_number = input("Which IC do you want to use as a template?" | ||
subject_id = input("Which participant ICA do you want" |
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.
variable renaming for consistency
|
||
return cleaned_epochs_ICA | ||
if (len(subject_id) == 0 or len(component_id) == 0): |
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.
Invert code flow to simplify code (return early to reduce nested code)
threshold=0.9, | ||
label='blink', | ||
ch_type='eeg') | ||
corrmap(icas, |
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.
Added plot
as ICA_apply
argument (to avoid plots launching GUI windows during tests). The presence/absence of plot
argument here change the return type of corrmap
, so we cannot always unpack fig_template
and fig_detected
. Anyway, they are not used.
|
||
from hypyp import prep | ||
|
||
def test_filt(): |
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.
new test
dict(method='fastica', fit_params=dict(tol=0.01)), # increase tolerance to converge | ||
dict(method='infomax', fit_params=dict(extended=True)) | ||
]) | ||
def test_ICA(epochs, fit_kwargs): |
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.
test moved from test_stats.py
- parameterize to test
fastica
- added amplitude comparison
- added the
plot=False
to avoid launching GUI windows
dict(strategy='union'), | ||
dict(strategy='intersection'), | ||
]) | ||
def test_AR_local(epochs, AR_local_kwargs): |
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.
test moved from test_stats.py
- parameterize for better coverage of
intersection
dict(strategy='union'), | ||
dict(strategy='intersection'), | ||
]) | ||
def test_AR_local_exception(epochs, AR_local_kwargs): |
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 is a working syntax for exception testing. The previous one was not testing the behavior properly
@@ -6,7 +6,6 @@ | |||
import numpy as np | |||
import scipy | |||
import mne | |||
from hypyp import prep |
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.
moved prep
related tests to test_prep.py
06e606b
to
52e9cd6
Compare
def filt(raw_S: list) -> list: | ||
def filt( | ||
raw_S: List[mne.io.Raw], | ||
freqs: Tuple[Union[float, None], Union[float, None]] = (2., 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.
added the "freqs" argument to complete the TODO in this function
Fixed issues:
Improvements:
freqs
argument toprep.filt()
prep.filt()