-
Notifications
You must be signed in to change notification settings - Fork 98
Handle channel name mismatches #1466
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
Handle channel name mismatches #1466
Conversation
|
Hello! 👋 Thanks for opening your first pull request here! |
for more information, see https://pre-commit.ci
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1466 +/- ##
==========================================
- Coverage 96.92% 96.92% -0.01%
==========================================
Files 43 43
Lines 10015 10074 +59
==========================================
+ Hits 9707 9764 +57
- Misses 308 310 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
larsoner
left a comment
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.
Otherwise I think it's good to go!
larsoner
left a comment
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.
@drammock feel free to merge if you're happy as well
|
Or actually @scott-huberty you have a "changes requested", can you re-review and approve if you're happy? |
enabled merged when green. Thanks @Kallemakela ! |
|
🎉 Congrats on merging your first pull request! 🥳 Looking forward to seeing more from you in the future! 💪 |
|
@Kallemakela thank you so much. I think we still need to add you to https://github.com/mne-tools/mne-bids/blob/main/CITATION.cff See also https://github.com/mne-tools/mne-bids/blob/main/CONTRIBUTING.md#instructions-for-first-time-contributors |
PR Description
Channels are renamed here without a warning or error when there is a mismatch between the raw data and BIDS
channels.tsv. This can cause a possible mismatch between channel names and the underlying locations (raw.info['chs'][i]['loc']) and signal data (raw._data).@larsoner proposed a fix here that was never implemented:
So I implemented it, except using
'raise'as the default. In my opinion, this makes more sense as this is a clear error in the BIDS dataset.Maybe we should also:
Related issues:
#1360
#1117
Merge checklist
Maintainer, please confirm the following before merging.
If applicable: