Skip to content

[WIP] Update tutorial to use BIDSDataGrabber #131

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

Merged
merged 4 commits into from
Oct 15, 2018
Merged

[WIP] Update tutorial to use BIDSDataGrabber #131

merged 4 commits into from
Oct 15, 2018

Conversation

adelavega
Copy link
Contributor

@adelavega adelavega commented Oct 10, 2018

I've gone through and updated basic_data_input_bids to use niype's BIDSDataGrabber interface. Check it out and let me know if it makes sense to you.

There are a few more changes to make (not all of which may make into this PR)

  • Expand tutorial a bit. There are a few more useful features of BIDSDataGrabber I may want to highlight. E.g. the defaults are quite usefunc (func & anat by default), and more complex queries are possible.
  • Fix outdated BIDS entities. E.g. modality is no longer a supported entity. It was a mistake to include it and according to BIDS spec its actually datatype
  • Maybe wait for updated to BIDSDataGrabber (for same reason as above, and to support derivatives, and new pybids syntax in 0.7)

Also, you're example of loading meta-data makes it obvious it would be quite useful if an interface made that easier (rather than having to wrap in a Function)

@adelavega
Copy link
Contributor Author

Oops, looks like some of my changes didn't save, so wait for those

@miykael
Copy link
Owner

miykael commented Oct 10, 2018

No problem. And don't worry about the circleci errors. The building of the docker image is currently broken but should be fixed quickly.

@adelavega
Copy link
Contributor Author

Okay, there we go. I had a weird issue with user permissions in Docker that didn't let me save any changes. It should now be updated

@miykael
Copy link
Owner

miykael commented Oct 10, 2018

Thank you @adelavega! Sorry for bothering you with this. But for easier reviewing reasons, could you commit the notebook with empty output cells? Such an approach also keeps the overall repo size small.

@adelavega
Copy link
Contributor Author

I also rewrote history to get rid of that data in the repo.

@adelavega
Copy link
Contributor Author

Did that work? I cleared outputs but now I don't see the "hide" button for Examples.

@miykael
Copy link
Owner

miykael commented Oct 10, 2018

That looks good. The "hide" button is not visible on github, but is still contained in the actual notebook.

@adelavega
Copy link
Contributor Author

adelavega commented Oct 15, 2018

hey @miykael , I refined the notebook a bit, and I think it's ready for your review/merge.

The upstream updates to pybids / nipype may take a while to release, so we might as merge this and update the notebooks down the road.

I think I'll be adding metadata support, and smarter iterables for BIDSDataGrabber

Small correction of the notebook metadata to keep it the same as the other notebooks.
@miykael
Copy link
Owner

miykael commented Oct 15, 2018

Hey @adelavega - thanks for the updates! This looks all fine to me and can be merged. The circleci errors are taken care of on master, so we don't have to worry about this.

An metadata support and smarter iterables for BIDSDataGrabber would be awesome! 👍

@adelavega
Copy link
Contributor Author

Sounds good! Go for it.

@miykael
Copy link
Owner

miykael commented Oct 15, 2018

Above, you mention the modality / datatype thematic. Will this come with pybids 0.7?

@adelavega
Copy link
Contributor Author

Yes, basically what happened is that although pybids use the term modality, and type, in the BIDS spec it was always datatype and suffix. This has been corrected in pybids 0.7, and I opened a PR to updated BIDSDataGrabber once pybids 0.7 is released: nipy/nipype#2737

We're waiting until this weeks BIDS-Model meeting at Stanford (and more testing to think about merging. We are hoping this is the least time we break the API ... at least in a while :)

@miykael
Copy link
Owner

miykael commented Oct 15, 2018

Hehe :-) That sounds good. Will merge this for now, and once pybids 0.7 comes out we can update this notebook again.

@miykael miykael merged commit 4b70cb1 into miykael:master Oct 15, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants