-
Notifications
You must be signed in to change notification settings - Fork 532
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
FIX,REF: BIDSDataGrabber #2336
FIX,REF: BIDSDataGrabber #2336
Conversation
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 am very in favor of this, the api change shouldn't be a huge deal since this is a relatively new interface but I'll let a second pair of eyes merge this.
One suggestion is adding this interface to the interface level __init__
for easier import?
https://github.com/nipy/nipype/blob/master/nipype/interfaces/__init__.py#L12
"anat": {"modality": "anat"}} | ||
|
||
# If infields is empty, use all BIDS entities | ||
if infields is None and have_pybids: |
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.
good catch
@@ -1,147 +0,0 @@ | |||
# -*- coding: utf-8 -*- |
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 am okay with this change - but removing this is an api change from 0.14
@satra WDYT
@mvdoc This would be good to get fixed before 1.0 (hoping to release tomorrow). |
!!! 1.0 !!! OK, will fix asap |
61116d2
to
3352ae6
Compare
* 'master' of github.com:nipy/nipype: fix error type remove install_aliases sty fix tests refactoring tests update changes [MAINT] Cleanup engine/base
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
Fixes #2333 .
Changes proposed in this pull request
BIDSDataGrabber
tointerfaces.io
for consistency, and also for usingIOBase
as a base class (otherwise failed for circular import)infields
was set, it was populated anyway by Pybids defaultsBIDSDataGrabber
is now subclass ofIOBase
instead ofBaseInterface