-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
[WIP] Add bids converter #4128
[WIP] Add bids converter #4128
Conversation
Codecov Report
@@ Coverage Diff @@
## master #4128 +/- ##
==========================================
- Coverage 86.05% 84.17% -1.88%
==========================================
Files 354 355 +1
Lines 63734 63772 +38
Branches 9709 9711 +2
==========================================
- Hits 54847 53683 -1164
- Misses 6216 7380 +1164
- Partials 2671 2709 +38 Continue to review full report at Codecov.
|
maybe we should put this in |
interested in this as well, let me know when you meet |
Funny PR that just adds code without calling it :) |
It's wip ;-)
…On Wed, Apr 5, 2017, 7:48 PM Denis A. Engemann ***@***.***> wrote:
Funny PR that just adds code without calling it :)
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#4128 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABwSHSkdRhRKS1x8iDmBn7jjxaDSWwnHks5rtFJrgaJpZM4Mt4FZ>
.
|
I know. But maybe some provocations can help changing the status :)
On Wed, Apr 5, 2017 at 10:53 PM Chris Holdgraf <notifications@github.com>
wrote:
… It's wip ;-)
On Wed, Apr 5, 2017, 7:48 PM Denis A. Engemann ***@***.***>
wrote:
> Funny PR that just adds code without calling it :)
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <
#4128 (comment)>,
> or mute the thread
> <
https://github.com/notifications/unsubscribe-auth/ABwSHSkdRhRKS1x8iDmBn7jjxaDSWwnHks5rtFJrgaJpZM4Mt4FZ
>
> .
>
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#4128 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AB0figdjRHgTvqtIQPOwB_6yEHZTp26hks5rtFOxgaJpZM4Mt4FZ>
.
|
I think some folks are still out of town etc, but I believe @jasmainak was going to iterate on this per the conversations we had at the sprint! |
@dengemann you will see me next week. Perhaps a good opportunity to push for this ;) |
yes we can push together :D
…On Thu, Apr 6, 2017 at 2:24 PM Mainak Jas ***@***.***> wrote:
@dengemann <https://github.com/dengemann> you will see me next week.
Perhaps a good opportunity to push for this ;)
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#4128 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AB0finkC_IcUNefbCCZj-1PlIpsswtVYks5rtS2vgaJpZM4Mt4FZ>
.
|
07db06c
to
53bc8c4
Compare
53bc8c4
to
817af82
Compare
I added a tiny example. Perhaps I could do with a round of reviews and words of encouragement to help me finish this PR ;) |
You mean the auto reject PR? I couldn't encourage you more :)
…On Sat, Apr 22, 2017 at 6:23 PM Mainak Jas ***@***.***> wrote:
I added a tiny example. Perhaps I could do with a round of reviews and
words of encouragement to help me finish this PR ;)
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#4128 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AB0fiuPD1visy4iBIQYRVXdGX5VEMWVoks5ryn3OgaJpZM4Mt4FZ>
.
|
|
||
fnames = dict(events='sample_audvis_raw-eve.fif', | ||
raw='sample_audvis_raw.fif') | ||
|
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.
I think it'd be good to have a little more explanation about what kind of folder structure BIDS follows. Maybe we could add in some RST that gave a rough idea of what this folder structure would be, something w/ this structure:
root/
anat/
meg/
file1.fif
etc
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.
yeah maybe eventually. For now, I'll try to provide a link to the BIDS draft ...
|
||
input_path = op.join(data_path, 'MEG', 'sample') | ||
output_path = op.join(data_path, '..', 'MNE-sample-data-bids') | ||
folder_to_bids(input_path=input_path, output_path=output_path, |
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.
Is there some minimal amount of information that each file must have, that is not absolutely required from MNE?
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.
yeah, there is. You have required fields and you have optional fields. There has to be way to indicate to the user that these required fields are incomplete even after the automatic conversion
|
||
input_path = op.join(data_path, 'MEG', 'sample') | ||
output_path = op.join(data_path, '..', 'MNE-sample-data-bids') | ||
folder_to_bids(input_path=input_path, output_path=output_path, |
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.
How about metadata for time etc? Does it only pull this from a trigger channel? What if we added in support for an events
array along with event_id
, and it could build an events TSV
file from this if it existed.
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.
Metadata for time comes from raw.info
. For the events, I'm reading it from an events
file for now. Didn't want to do too much guessing about trigger channel etc. ...
@jasmainak I would create a mne-bids repo in mne-tools to quickly make progress. Working in the mne-python code base will be more difficult. wdyt? |
@agramfort that's an interesting idea. What can we imagine would go in there other than just this PR? Maybe a place to host the MNE-BIDS data? I agree this might be a good offshoot project for now, just trying to envision what it'd look like. |
I don't know but MEG bids is still evolving fast and a PR is not ideal for
quick collab without nitpicks.
|
yeah I agree. Let's make a I think apart from the MNE sample data, I'd like to see how much of the conversion for the openfmri data we can easily automate. I feel that with |
here we go:
https://github.com/mne-tools/mne-bids
|
sounds good to me - I'm working on a BIDS-iEEG draft w/ some colleagues as well, so I can push some of our sample data there too perhaps! |
cool, I added what's in this PR to the repo. Let's iterate from there |
closes #4127
The BIDS specification can be found here
The BIDS compatible MNE sample data can be found here. Let's see how much of this we can automate.
cc @choldgraf @teonbrooks @dengemann