MRG: Test warnings if NIRS channels are not correctly ordered #7491
Conversation
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
|
@larsoner @agramfort could you please review this. Thanks The failing CI test seems unrelated |
rob-luke
changed the title
agramfort
reviewed
Mar 22, 2020
mne/preprocessing/nirs/nirs.py
Outdated
| (int(ch2_name_info.groups()[2]) != freqs[1]): | ||
| raise RuntimeError('NIRS channels not ordered correctly') | ||
| else: | ||
| raise RuntimeError('NIRS channels not ordered correctly') |
There was a problem hiding this comment.
I would have used a ValueError and I would give more details on the error eg printing the problematic values. YOu could also avoid copy pasting the error message. besides LGTM
Codecov Report
@@ Coverage Diff @@
## master #7491 +/- ##
==========================================
- Coverage 90.09% 90.05% -0.04%
==========================================
Files 453 453
Lines 82608 82642 +34
Branches 13060 13049 -11
==========================================
- Hits 74424 74422 -2
- Misses 5358 5391 +33
- Partials 2826 2829 +3 |
agramfort
approved these changes
Mar 22, 2020
|
Thanks @agramfort. Sorry but I tweaked the error messages again after you approved the PR. I tried to better match other code in MNE. |
agramfort
approved these changes
Mar 22, 2020
|
Pushed a cosmit to reduce the indentation level, will merge once CIs come back happy -- thanks @rob-luke |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Reference issue
General fNIRS tweaking #7057
What does this implement/fix?
Added test to ensure that NIRS data is stored as paired wavelength data. Noticed that if an odd number of channels existed that the check failed.