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
MRG: Add support for NIRS to plot_topomap #7063
MRG: Add support for NIRS to plot_topomap #7063
Changes from 4 commits
d0ec2b2
69158a5
b382e8a
b71f10c
6d6322a
0c4421c
8590af8
bdf2916
7174e66
f75d74e
add05f3
a9e7a4d
5f0ebce
509f30d
2d1da2c
4f673bd
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.
_clean_names is used in a number of places. I would rather avoid the branching here to make sure it's consistent with other places. How specific are NIRS channel names? What ch name breaks 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.
The name given by the device manufacturer is in the format of source number then detector number, so for example “S1 D2” is the channel between source one and detector two. We then appended the light frequency or chomaphore. So currently the MNE channel names are in the format
S1 D2 760
.The problem was that the spaces were getting removed and the first characters were being kept, so we would have several channel names that were modified to be “S1” for example (because source one may have channels to several detectors). And that caused the error listed in the first comment of this PR.
But we can change the way channel names are stored. We could make it
S1-D2-760
OrS1D2-760
etc. I think this would solve the problem, but not sure if it will break other things. What do you think?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.
Sorry that’s wrong, the format is currently
S1-D2 760
. Another example isS12-D3 850
https://github.com/mne-tools/mne-python/blob/master/mne/io/nirx/tests/test_nirx.py#L37
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.
Ok I have push an attempt to change the naming to
S1_D3 760
. This way the source-detector pair is read as a single word, so stays unique without modifying clean_names. The raw.plot doesnt look as good, but I think thats ok. Thoughts?