Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[MRG] read_raw_bids to read in data from participants.tsv file #392

Merged
merged 23 commits into from Apr 23, 2020
Merged

[MRG] read_raw_bids to read in data from participants.tsv file #392

merged 23 commits into from Apr 23, 2020

Conversation

adam2392
Copy link
Member

PR Description

Closes: #391

Merge checklist

Maintainer, please confirm the following before merging:

  • All comments resolved
  • This is not your own PR
  • All CIs are happy
  • PR title starts with [MRG]
  • whats_new.rst is updated
  • PR description includes phrase "closes <#issue-number>"
  • Commit history does not contain any merge commits

@adam2392 adam2392 changed the title Participants [WIP] read_raw_bids to read in data from participants.tsv file Apr 17, 2020
@codecov-io
Copy link

codecov-io commented Apr 17, 2020 •
edited

Codecov Report

Merging #392 into master will increase coverage by 0.11%.
The diff coverage is 96.77%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #392      +/-   ##
==========================================
+ Coverage   95.19%   95.30%   +0.11%     
==========================================
  Files          11       11              
  Lines        1373     1427      +54     
==========================================
+ Hits         1307     1360      +53     
- Misses         66       67       +1
Impacted Files Coverage Δ
mne_bids/config.py 93.33% <85.71%> (-6.67%) ⬇️
mne_bids/read.py 97.85% <100.00%> (+0.25%) ⬆️
mne_bids/utils.py 98.43% <100.00%> (+0.09%) ⬆️
mne_bids/write.py 97.70% <100.00%> (+0.17%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3ddf7a5...12f9994. Read the comment docs.

agramfort
agramfort reviewed Apr 17, 2020
View reviewed changes
Copy link
Member

@agramfort agramfort left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks clean.

@sappelhoff or @jasmainak can you have a look?

sappelhoff
sappelhoff reviewed Apr 17, 2020
View reviewed changes
Copy link
Member

@sappelhoff sappelhoff left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM apart from my two comments 👍

mne_bids/read.py Outdated Show resolved Hide resolved
mne_bids/read.py Outdated Show resolved Hide resolved
@adam2392
Copy link
Member Author

Added unit test, and fix on bday from "age".

jasmainak
jasmainak reviewed Apr 17, 2020
View reviewed changes
mne_bids/utils.py Outdated Show resolved Hide resolved
@adam2392 adam2392 requested a review from jasmainak April 17, 2020 17:59
@adam2392 adam2392 changed the title [WIP] read_raw_bids to read in data from participants.tsv file [MRG] read_raw_bids to read in data from participants.tsv file Apr 17, 2020
agramfort
agramfort reviewed Apr 20, 2020
View reviewed changes
mne_bids/read.py Outdated Show resolved Hide resolved
mne_bids/tests/test_read.py Outdated Show resolved Hide resolved
mne_bids/utils.py Outdated Show resolved Hide resolved
mne_bids/utils.py Outdated Show resolved Hide resolved
jasmainak
jasmainak reviewed Apr 21, 2020
View reviewed changes
mne_bids/utils.py Outdated Show resolved Hide resolved
@agramfort
Copy link
Member

agramfort commented Apr 21, 2020 via email

@jasmainak
Copy link
Member

jasmainak commented Apr 21, 2020 •
edited

it's a pity you can't attach metadata to the raw object like the epochs object, otherwise this might have been the perfect candidate

never mind :)

@agramfort
Copy link
Member

agramfort commented Apr 21, 2020 via email

@jasmainak
Copy link
Member

I'm thinking even if you wanted to do age prediction, it's easier to read the participants.tsv file using pandas than to recompute the age from the measurement date and birthday. Also imagine if it's anonymized this won't work ...

@agramfort
Copy link
Member

agramfort commented Apr 21, 2020 via email

agramfort
agramfort reviewed Apr 22, 2020
View reviewed changes
mne_bids/tests/test_read.py Outdated Show resolved Hide resolved
mne_bids/tests/test_read.py Outdated Show resolved Hide resolved
@agramfort agramfort merged commit 809c9db into mne-tools:master Apr 23, 2020
@agramfort
Copy link
Member

thx @adam2392

@jasmainak jasmainak added this to In progress in 0.5 via automation Apr 25, 2020
@jasmainak jasmainak moved this from In progress to Reviewer approved in 0.5 Apr 25, 2020
@jasmainak jasmainak moved this from Reviewer approved to Done in 0.5 Apr 25, 2020
@hoechenberger hoechenberger mentioned this pull request Apr 27, 2020
Closed
@adam2392 adam2392 deleted the participants branch April 27, 2020 21:54
@hoechenberger hoechenberger mentioned this pull request Apr 29, 2020
Closed
@sappelhoff sappelhoff mentioned this pull request May 28, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@agramfort agramfort agramfort left review comments

@jasmainak jasmainak Awaiting requested review from jasmainak

@sappelhoff sappelhoff Awaiting requested review from sappelhoff

Assignees
No one assigned
Labels
None yet
Projects
No open projects
0.5
  
Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Have read_raw_bids read in the data in participants.tsv
5 participants
@adam2392 @codecov-io @agramfort @jasmainak @sappelhoff