-
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
ENH: Add ability to reject epochs using callables #12195
Changes from 13 commits
81ef83e
d8dda07
e9294de
06e6770
867496d
1b4f5b3
2a66049
e16f4a1
e708465
fbdec77
6e23ecc
cdd2843
cd9f7b1
3f5fd84
a74ccf6
f0cb1b8
7d02fca
1b836ad
f3e8841
984c604
fbe4cd2
ee599a7
24e669c
bce6486
8401c92
cf1facf
e98bee2
a579491
c9da4db
5a7a618
98b92c4
44fdf8a
3ff8a9e
e729e81
fd4c75f
a685b89
65778d1
3ece314
db0cf11
b2686c0
3ae37a4
45b30f3
e77ae63
f560ccd
235f6d3
4e4b369
b7c6a36
ba45b5a
b5829b0
a11a410
b918ea8
ab009bc
8cf2e49
fb6a5b1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -819,9 +819,14 @@ def _reject_setup(self, reject, flat): | |
# check for invalid values | ||
for rej, kind in zip((reject, flat), ("Rejection", "Flat")): | ||
for key, val in rej.items(): | ||
if val is None or val < 0: | ||
if callable(val): | ||
continue | ||
elif val is not None and val >= 0: | ||
continue | ||
else: | ||
raise ValueError( | ||
'%s value must be a number >= 0, not "%s"' % (kind, val) | ||
"%s value must be a number >= 0 or a valid function," | ||
'not "%s"' % (kind, val) | ||
) | ||
|
||
# now check to see if our rejection and flat are getting more | ||
larsoner marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
@@ -834,33 +839,48 @@ def _reject_setup(self, reject, flat): | |
"previous ones" | ||
) | ||
|
||
# Skip this check if old_reject, reject, old_flat, and flat are | ||
# callables | ||
is_callable = False | ||
for rej in (reject, flat, old_reject, old_flat): | ||
for key, val in rej.items(): | ||
if callable(val): | ||
is_callable = True | ||
|
||
# copy thresholds for channel types that were used previously, but not | ||
# passed this time | ||
for key in set(old_reject) - set(reject): | ||
reject[key] = old_reject[key] | ||
# make sure new thresholds are at least as stringent as the old ones | ||
for key in reject: | ||
if key in old_reject and reject[key] > old_reject[key]: | ||
raise ValueError( | ||
bad_msg.format( | ||
kind="reject", | ||
key=key, | ||
new=reject[key], | ||
old=old_reject[key], | ||
op=">", | ||
|
||
if not is_callable: | ||
# make sure new thresholds are at least as stringent | ||
# as the old ones | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ... but I think you can make these checks more fine-grained without much more work. Instead of if any are callable, skip all checks (which is what you're doing now), you should within this loop be able to fairly easily make it so that if this type is or was callable, skip this check. I think it almost amounts to moving the conditional from above into this loop with a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should I edit my test to check for this? |
||
for key in reject: | ||
if key in old_reject and reject[key] > old_reject[key]: | ||
raise ValueError( | ||
bad_msg.format( | ||
kind="reject", | ||
key=key, | ||
new=reject[key], | ||
old=old_reject[key], | ||
op=">", | ||
) | ||
) | ||
) | ||
|
||
# same for flat thresholds | ||
for key in set(old_flat) - set(flat): | ||
flat[key] = old_flat[key] | ||
for key in flat: | ||
if key in old_flat and flat[key] < old_flat[key]: | ||
raise ValueError( | ||
bad_msg.format( | ||
kind="flat", key=key, new=flat[key], old=old_flat[key], op="<" | ||
# same for flat thresholds | ||
for key in set(old_flat) - set(flat): | ||
flat[key] = old_flat[key] | ||
for key in flat: | ||
if key in old_flat and flat[key] < old_flat[key]: | ||
raise ValueError( | ||
bad_msg.format( | ||
kind="flat", | ||
key=key, | ||
new=flat[key], | ||
old=old_flat[key], | ||
op="<", | ||
) | ||
) | ||
) | ||
|
||
# after validation, set parameters | ||
self._bad_dropped = False | ||
|
@@ -3621,25 +3641,35 @@ def _is_good( | |
): | ||
"""Test if data segment e is good according to reject and flat. | ||
|
||
The reject and flat dictionaries can accept functions as values. | ||
withmywoessner marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
If full_report=True, it will give True/False as well as a list of all | ||
offending channels. | ||
""" | ||
bad_tuple = tuple() | ||
has_printed = False | ||
checkable = np.ones(len(ch_names), dtype=bool) | ||
checkable[np.array([c in ignore_chs for c in ch_names], dtype=bool)] = False | ||
|
||
for refl, f, t in zip([reject, flat], [np.greater, np.less], ["", "flat"]): | ||
if refl is not None: | ||
for key, thresh in refl.items(): | ||
for key, criterion in refl.items(): | ||
idx = channel_type_idx[key] | ||
name = key.upper() | ||
if len(idx) > 0: | ||
e_idx = e[idx] | ||
deltas = np.max(e_idx, axis=1) - np.min(e_idx, axis=1) | ||
checkable_idx = checkable[idx] | ||
idx_deltas = np.where( | ||
np.logical_and(f(deltas, thresh), checkable_idx) | ||
)[0] | ||
|
||
# Check if criterion is a function and apply it | ||
if callable(criterion): | ||
idx_deltas = np.where( | ||
np.logical_and(criterion(e_idx), checkable_idx) | ||
)[0] | ||
else: | ||
deltas = np.max(e_idx, axis=1) - np.min(e_idx, axis=1) | ||
idx_deltas = np.where( | ||
np.logical_and(f(deltas, criterion), checkable_idx) | ||
)[0] | ||
|
||
if len(idx_deltas) > 0: | ||
bad_names = [ch_names[idx[i]] for i in idx_deltas] | ||
|
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.
I would simplify to:
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.
I was going to do it this way, but it breaks a test. I think its because
_validate_type
returns a TypeError and Pytest wants a ValueError: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.
We should just update the test, TypeError is a better error in the case where the type is wrong (i.e.,
_validate_type
does something better than what we do inmain
)