-
Notifications
You must be signed in to change notification settings - Fork 15
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] Moredocs and experiment with api implementation #31
Merged
Changes from 4 commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
4b93543
Adding details to docs
adam2392 8964a5e
Exp w/ transformer
adam2392 218556c
Fixing API
adam2392 7c798e0
fix import
mscheltienne 01f1128
Fix docs
adam2392 87fd7bf
Merge branch 'moredocs' of https://github.com/adam2392/mne-icalabel i…
adam2392 26a2123
Fix example
adam2392 144c6ad
Fix example
adam2392 f83f0db
Fixing example and style
adam2392 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
ICLABEL_NUMERICAL_TO_STRING = { | ||
0: "brain", | ||
1: "muscle artifact", | ||
2: "eye blink", | ||
3: "heart beat", | ||
4: "line noise", | ||
5: "channel noise", | ||
6: "other", | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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.
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.
Any reason for nesting the output in a dictionary? Can't we directly have
return y_pred_proba, y_pred, labels
(in this order or another)?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.
No real preference.
I only did this to make it super explicit and limit the number of return variables / make it a one-liner to convert to a Dataframe. Would you prefer the 3 tuple return instead?
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.
No strong preference either, but you can't convert this to a DataFrame that easily either since
labels_pred_proba
is a 2D array.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.
Ah fair. Hmm now that I think about it more I do prefer in general single return arguments, but I'm also unsure if this is optimal here... I'll leave this open for now and we can always change it.
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 like the idea of simple conversion to DataFrame for the user (of course not on our end, let's not have pandas as a dependency just for that). With that in mind, I see 2 individual DataFrame in the proposed output:
and:
So how about 2 outputs:
return dict(labels=labels, y_pred=labels_pred), {value: labels_pred_proba[:, k] for k, value in ICLABEL_NUMERICAL_TO_STRING.items()}
.IMO, for what needs to be stored in the mne ICA instance, we do not need the
y_pred
, but only thelabels
and the second dict.With this solution, you can get the 'raw' numpy array of probabilities with
mne_icalabel.iclabel.iclabel_label_compoments
or dictionaries ready to be transformed to DataFrames withmne_icalabel.label_components
.WDYT?