Fix bug if no event descriptions are present; support stim_type column in events.tsv #680
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
If no event descriptions could be generated, we'd crash as we'd try to access descriptions like an array, while our placeholder would only be a string. This is fixed now. The other part of this PR is related to the naming of the column in the events.tsv file that contains the event description. According to BIDS, this column is optional and shall be named `trial_type`. Now I've come across multiple (older?) datasets where the column is called `stim_type` instead. I've now added support for reading this column if `stim_type` is not present. A warning will be issued in this case, informing the user that the column name should be changed. This was necessary to read the ds000117 datset from https://openneuro.org/datasets/ds000117
hoechenberger
changed the title
hoechenberger
added a commit
to hoechenberger/mne-bids-pipeline
that referenced
this pull request
Jan 14, 2021
Closes mne-tools#41 Remaining / highlightes issues: - Original dataset doesn't store fine-calibration and crosstalk files according to BIDS, so not doing Maxwell filtering for now - I'd like to calculate the contrast of "face" vs "scrambled", however, to do that, I'd need to merge the "familiar" and "unfamiliar" events to "faces". We need to add support for operations like this. Tests will fail until mne-tools/mne-bids#680 has been merged.
hoechenberger
changed the title
Codecov Report
@@ Coverage Diff @@
## master #680 +/- ##
==========================================
+ Coverage 93.71% 93.80% +0.08%
==========================================
Files 22 22
Lines 2707 2713 +6
==========================================
+ Hits 2537 2545 +8
+ Misses 170 168 -2
Continue to review full report at Codecov.
|
sappelhoff
approved these changes
Jan 14, 2021
|
1 job failing due to download problems. |
agramfort
approved these changes
Jan 14, 2021
|
Since the download problem seems to be persistent (@adam2392 was running into this issue too), and given that all but that one CI runs are green, this should be good to merge. |
|
thanks @hoechenberger |
agramfort
pushed a commit
to mne-tools/mne-bids-pipeline
that referenced
this pull request
Jan 14, 2021
* Process Faces dataset Closes #41 Remaining / highlightes issues: - Original dataset doesn't store fine-calibration and crosstalk files according to BIDS, so not doing Maxwell filtering for now - I'd like to calculate the contrast of "face" vs "scrambled", however, to do that, I'd need to merge the "familiar" and "unfamiliar" events to "faces". We need to add support for operations like this. Tests will fail until mne-tools/mne-bids#680 has been merged. * Remove cruft
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.
If no event descriptions could be generated, we'd crash as we'd try
to access descriptions like an array, while our placeholder would only
be a string. This is fixed now.
The other part of this PR is related to the naming of the column in the
events.tsv file that contains the event description. According to
BIDS, this column is optional and shall be named
trial_type. NowI've come across multiple (older?) datasets where the column is
called
stim_typeinstead. I've now added support for reading thiscolumn if
trial_typeis not present. A warning will be issued inthis case, informing the user that the column name should be changed.
This was necessary to read the ds000117 datset from
https://openneuro.org/datasets/ds000117
Merge checklist
Maintainer, please confirm the following before merging: