Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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, MRG] Allow epoch construction from annotations #12311
[ENH, MRG] Allow epoch construction from annotations #12311
Changes from 30 commits
4fa044e
eb73298
daee067
4447ed8
24270d1
7b5b9d8
d4bc5c9
1378048
241e80b
3bc03fb
c2098a4
47cc039
5776741
3914d0b
029d131
a44554a
7391b0f
f102534
bf97c0a
57477b5
5b9e571
517c789
1290aac
4c46e11
27615c8
7db0b59
fb87172
7ef087d
c3ae450
f9401bd
cad31f4
2c33dab
52542ea
135f523
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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 put this block in a private function to be clear about what it needs and returns
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.
+1, the
__init__
is already long enough as it is and a separate function would help with thatThere 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.
The logic here should probably mimic what we do already for missing event IDs, e.g.,. having
Epochs(raw, event_id=["auditory", "visual"], on_missing="ignore")
should be okay when there are only"auditory"
events in the annotations. Maybe you can remove some of the logic here to allow the existingevent_id+on_missing
checking/logic to take care of some stuff? (And please add a test for this case!)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.
It looks like this comment was not addressed yet and was errantly resolved by GitHub because the code was moved
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.
Ahh I see there is an
_on_missing
in there now, never mind! Maybe we could deduplicate more at some point (?) but seems okay for now