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] Adding status_description
to channels.tsv files defaulting to n/a for now
#489
Conversation
Codecov Report
@@ Coverage Diff @@
## master #489 +/- ##
=======================================
Coverage 92.79% 92.79%
=======================================
Files 14 14
Lines 1970 1971 +1
=======================================
+ Hits 1828 1829 +1
Misses 142 142
Continue to review full report at Codecov.
|
I missed the discussion in #348, but since I've started working on #491 I'm wondering why you would want to fill it with |
I think either value works, so can change it. That should pass the bids-validator right? |
Let's see what the others think :)
Hah, thanks for the reminder. I forgot to include BIDS validation in #491. Let's see… |
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.
LGTM!
@sappelhoff Feel free to merge if you're happy |
+0 on my side. I did not follow discussion. @sappelhoff @jasmainak ball is in your hands |
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.
LGTM -> doesn't hurt and will come in helpful when we have TSV + JSON updating functions
won't pass the validation with an empty cell, so I'm using n/a
yes, n/a
is good here. 👍
thanks @adam2392 |
PR Description
Closes: #348
Defaults to
n/a
for now to allow for easy grepping as @jasmainak suggested. Future improvements can be made that allow the status to change, pending an addition to mne-python, or implementation of #458Merge checklist
Maintainer, please confirm the following before merging: