-
Notifications
You must be signed in to change notification settings - Fork 185
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
More natural phrasing in README.rst #396
Conversation
Codecov Report
@@ Coverage Diff @@
## master #396 +/- ##
=======================================
Coverage 82.63% 82.63%
=======================================
Files 53 53
Lines 3738 3738
=======================================
Hits 3089 3089
Misses 649 649 |
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.
Your changes look great to me! But, you may need to change also the index.rst to affect the new webpage.
Also, I think we have support for ECoG because of MNE, example: https://braindecode.org/auto_examples/plot_bcic_iv_4_ecog_trial.html
README.rst
Outdated
`Braindecode`_ is an open-source Python toolbox for decode neurophysiological data such as MEG, EEG, sEEG, | ||
ECoG, and more with Deep Learning models. It includes modules for data input/output, preprocessing, | ||
visualization, data augmentation, classification, regression, sampler and much more! | ||
`Braindecode`_ is an open-source Python toolbox for decoding raw EEG data with |
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 keep ECoG as we have two examples analyzing ECoG data, for example this one: plot_bcic_iv_4_ecog_cropped.py
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.
Sure! Didn't know about those 😅 Cool stuff!
I made a small commit to change this ECoG and MEG issue (because of the MNE), and to copy the information to index.rst for the new documentation. @sliwy or @cedricrommel, for me you can apply the merge in PR. |
Sorry guys, I am a bit taken by the NeurIPS rebuttal phase. I would recommending not merging this PR yet as I found some other copy-paste mistakes in the FAQ if I remember well. Would be nice to gather all these small fixes in this PR. Will be back actively next week! |
I'm not so sure about the changes removing EEG/MEG etc, like brain signals can also be fMRI, which we don't really do at all so far? |
Co-authored-by: Alexandre Gramfort <alexandre.gramfort@m4x.org>
Co-authored-by: Alexandre Gramfort <alexandre.gramfort@m4x.org>
Co-authored-by: robintibor <robintibor@gmail.com>
Co-authored-by: robintibor <robintibor@gmail.com>
Alright guys, I just fixed the few things I saw but did not have time to deal with last week :) Good to go as far as it concerns me! |
yes looks good to me what do others think? |
Looks good to me! |
Merging it then! |
Sorry, I did not see this when the PR was opened, so just creating a new PR 😬
These are just small changes to the new README.rst in places where I found the phrasing a bit unnatural. Also, I removed the part mentioning the decoding of MEG, SEG and ECoG, since the lib does not support this other data modalities yet AFAIK.