-
Notifications
You must be signed in to change notification settings - Fork 533
[ENH] Update BIDSDataGrabber for pybids 0.7 #2737
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
Changes from all commits
ffd0d8e
8db5111
801b295
1d267c8
b0121c0
cc89c58
b8aacce
a0e92f1
0f94d9d
77a0510
26a57a6
2a84cab
5590d4d
3c21466
d2e53fb
b75bf7c
7105cb5
09a1a43
75a5a2b
139c163
b623cdd
aeda06a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,65 @@ | ||
# AUTO-GENERATED by tools/checkspecs.py - DO NOT EDIT | ||
from __future__ import unicode_literals | ||
from ..epi import EddyQuad | ||
|
||
|
||
def test_EddyQuad_inputs(): | ||
input_map = dict( | ||
args=dict(argstr='%s', ), | ||
base_name=dict( | ||
argstr='%s', | ||
position=0, | ||
usedefault=True, | ||
), | ||
bval_file=dict( | ||
argstr='--bvals=%s', | ||
mandatory=True, | ||
), | ||
bvec_file=dict(argstr='--bvecs=%s', ), | ||
environ=dict( | ||
nohash=True, | ||
usedefault=True, | ||
), | ||
field=dict(argstr='--field=%s', ), | ||
idx_file=dict( | ||
argstr='--eddyIdx=%s', | ||
mandatory=True, | ||
), | ||
mask_file=dict( | ||
argstr='--mask=%s', | ||
mandatory=True, | ||
), | ||
output_dir=dict( | ||
argstr='--output-dir=%s', | ||
name_source=['base_name'], | ||
name_template='%s.qc', | ||
), | ||
output_type=dict(), | ||
param_file=dict( | ||
argstr='--eddyParams=%s', | ||
mandatory=True, | ||
), | ||
slice_spec=dict(argstr='--slspec=%s', ), | ||
verbose=dict(argstr='--verbose', ), | ||
) | ||
inputs = EddyQuad.input_spec() | ||
|
||
for key, metadata in list(input_map.items()): | ||
for metakey, value in list(metadata.items()): | ||
assert getattr(inputs.traits()[key], metakey) == value | ||
def test_EddyQuad_outputs(): | ||
output_map = dict( | ||
avg_b0_pe_png=dict(), | ||
avg_b_png=dict(), | ||
clean_volumes=dict(), | ||
cnr_png=dict(), | ||
qc_json=dict(), | ||
qc_pdf=dict(), | ||
residuals=dict(), | ||
vdm_png=dict(), | ||
) | ||
outputs = EddyQuad.output_spec() | ||
|
||
for key, metadata in list(output_map.items()): | ||
for metakey, value in list(metadata.items()): | ||
assert getattr(outputs.traits()[key], metakey) == value |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -2721,18 +2721,23 @@ def _list_outputs(self): | |||||
|
||||||
|
||||||
class BIDSDataGrabberInputSpec(DynamicTraitedSpec): | ||||||
base_dir = Directory(exists=True, | ||||||
desc='Path to BIDS Directory.', | ||||||
mandatory=True) | ||||||
output_query = traits.Dict(key_trait=Str, | ||||||
value_trait=traits.Dict, | ||||||
desc='Queries for outfield outputs') | ||||||
raise_on_empty = traits.Bool(True, usedefault=True, | ||||||
desc='Generate exception if list is empty ' | ||||||
'for a given field') | ||||||
return_type = traits.Enum('file', 'namedtuple', usedefault=True) | ||||||
strict = traits.Bool(desc='Return only BIDS "proper" files (e.g., ' | ||||||
'ignore derivatives/, sourcedata/, etc.)') | ||||||
base_dir = Directory( | ||||||
exists=True, | ||||||
desc='Path to BIDS Directory.', | ||||||
mandatory=True) | ||||||
output_query = traits.Dict( | ||||||
key_trait=Str, | ||||||
value_trait=traits.Dict, | ||||||
desc='Queries for outfield outputs') | ||||||
raise_on_empty = traits.Bool( | ||||||
True, usedefault=True, | ||||||
desc='Generate exception if list is empty for a given field') | ||||||
index_derivatives = traits.Bool( | ||||||
False, mandatory=True, usedefault=True, | ||||||
desc='Index derivatives/ sub-directory') | ||||||
extra_derivatives = traits.List( | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. when using the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
(Requires importing |
||||||
Directory(exists=True), | ||||||
desc='Additional derivative directories to index') | ||||||
|
||||||
|
||||||
class BIDSDataGrabber(LibraryBaseInterface, IOBase): | ||||||
|
@@ -2758,10 +2763,10 @@ class BIDSDataGrabber(LibraryBaseInterface, IOBase): | |||||
are filtered on common entities, which can be explicitly defined as | ||||||
infields. | ||||||
|
||||||
>>> bg = BIDSDataGrabber(infields = ['subject'], outfields = ['dwi']) | ||||||
>>> bg = BIDSDataGrabber(infields = ['subject']) | ||||||
>>> bg.inputs.base_dir = 'ds005/' | ||||||
>>> bg.inputs.subject = '01' | ||||||
>>> bg.inputs.output_query['dwi'] = dict(modality='dwi') | ||||||
>>> bg.inputs.output_query['dwi'] = dict(datatype='dwi') | ||||||
>>> results = bg.run() # doctest: +SKIP | ||||||
|
||||||
""" | ||||||
|
@@ -2781,18 +2786,17 @@ def __init__(self, infields=None, **kwargs): | |||||
|
||||||
if not isdefined(self.inputs.output_query): | ||||||
self.inputs.output_query = { | ||||||
"func": {"modality": "func", 'extensions': ['nii', '.nii.gz']}, | ||||||
"anat": {"modality": "anat", 'extensions': ['nii', '.nii.gz']}, | ||||||
"bold": {"datatype": "func", "suffix": "bold", | ||||||
"extensions": ["nii", ".nii.gz"]}, | ||||||
"T1w": {"datatype": "anat", "suffix": "T1w", | ||||||
"extensions": ["nii", ".nii.gz"]}, | ||||||
} | ||||||
|
||||||
# If infields is empty, use all BIDS entities | ||||||
if infields is None: | ||||||
# Version resilience | ||||||
try: | ||||||
from bids import layout as bidslayout | ||||||
except ImportError: | ||||||
from bids import grabbids as bidslayout | ||||||
bids_config = join(dirname(bidslayout.__file__), 'config', 'bids.json') | ||||||
from bids import layout as bidslayout | ||||||
bids_config = join( | ||||||
dirname(bidslayout.__file__), 'config', 'bids.json') | ||||||
bids_config = json.load(open(bids_config, 'r')) | ||||||
infields = [i['name'] for i in bids_config['entities']] | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we replace all of this with There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That command ( I do not know of a better way (but I didn't look extremely hard) Another comment: I think it would be nice to include There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh, sorry, I meant that in a pseudocode sense of the entities attribute of an instantiated object, not that that snippet would work. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, maybe I can reorganize this to set up the layout first, before setting the output traits. Is there any problem with instantiating the layout in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The biggest problem is that inputs can change between instantiation and running. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's a pretty big problem! So I think for now instantiating a layout just to get entities does not work. Adding a helper function to pybids and then changing this later is a better option. |
||||||
|
||||||
|
@@ -2807,15 +2811,12 @@ def __init__(self, infields=None, **kwargs): | |||||
self.inputs.trait_set(trait_change_notify=False, **undefined_traits) | ||||||
|
||||||
def _list_outputs(self): | ||||||
# Version resilience | ||||||
try: | ||||||
from bids import BIDSLayout | ||||||
except ImportError: | ||||||
from bids.grabbids import BIDSLayout | ||||||
exclude = None | ||||||
if self.inputs.strict: | ||||||
exclude = ['derivatives/', 'code/', 'sourcedata/'] | ||||||
layout = BIDSLayout(self.inputs.base_dir, exclude=exclude) | ||||||
from bids import BIDSLayout | ||||||
layout = BIDSLayout(self.inputs.base_dir, | ||||||
derivatives=self.inputs.index_derivatives) | ||||||
|
||||||
if isdefined(self.inputs.extra_derivatives): | ||||||
layout.add_derivatives(self.inputs.extra_derivatives) | ||||||
|
||||||
# If infield is not given nm input value, silently ignore | ||||||
filters = {} | ||||||
|
@@ -2828,7 +2829,7 @@ def _list_outputs(self): | |||||
for key, query in self.inputs.output_query.items(): | ||||||
args = query.copy() | ||||||
args.update(filters) | ||||||
filelist = layout.get(return_type=self.inputs.return_type, **args) | ||||||
filelist = layout.get(return_type='file', **args) | ||||||
if len(filelist) == 0: | ||||||
msg = 'Output key: %s returned no files' % key | ||||||
if self.inputs.raise_on_empty: | ||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -581,8 +581,6 @@ def test_jsonsink(tmpdir, inputs_attributes): | |
# There are three reasons these tests will be skipped: | ||
@pytest.mark.skipif(not have_pybids, | ||
reason="Pybids is not installed") | ||
@pytest.mark.skipif(sys.version_info < (3, 0), | ||
reason="Pybids no longer supports Python 2") | ||
@pytest.mark.skipif(not dist_is_editable('pybids'), | ||
reason="Pybids is not installed in editable mode") | ||
def test_bids_grabber(tmpdir): | ||
|
@@ -591,31 +589,27 @@ def test_bids_grabber(tmpdir): | |
bg.inputs.base_dir = os.path.join(datadir, 'ds005') | ||
bg.inputs.subject = '01' | ||
results = bg.run() | ||
assert 'sub-01_T1w.nii.gz' in map(os.path.basename, results.outputs.anat) | ||
assert 'sub-01_T1w.nii.gz' in map(os.path.basename, results.outputs.T1w) | ||
assert 'sub-01_task-mixedgamblestask_run-01_bold.nii.gz' in \ | ||
map(os.path.basename, results.outputs.func) | ||
map(os.path.basename, results.outputs.bold) | ||
|
||
|
||
@pytest.mark.skipif(not have_pybids, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we remove the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah I guess... |
||
reason="Pybids is not installed") | ||
@pytest.mark.skipif(sys.version_info < (3, 0), | ||
reason="Pybids no longer supports Python 2") | ||
@pytest.mark.skipif(not dist_is_editable('pybids'), | ||
reason="Pybids is not installed in editable mode") | ||
def test_bids_fields(tmpdir): | ||
tmpdir.chdir() | ||
bg = nio.BIDSDataGrabber(infields = ['subject'], outfields = ['dwi']) | ||
bg.inputs.base_dir = os.path.join(datadir, 'ds005') | ||
bg.inputs.subject = '01' | ||
bg.inputs.output_query['dwi'] = dict(modality='dwi') | ||
bg.inputs.output_query['dwi'] = dict(datatype='dwi') | ||
results = bg.run() | ||
assert 'sub-01_dwi.nii.gz' in map(os.path.basename, results.outputs.dwi) | ||
|
||
|
||
@pytest.mark.skipif(not have_pybids, | ||
reason="Pybids is not installed") | ||
@pytest.mark.skipif(sys.version_info < (3, 0), | ||
reason="Pybids no longer supports Python 2") | ||
@pytest.mark.skipif(not dist_is_editable('pybids'), | ||
reason="Pybids is not installed in editable mode") | ||
def test_bids_infields_outfields(tmpdir): | ||
|
@@ -633,9 +627,9 @@ def test_bids_infields_outfields(tmpdir): | |
for outfield in outfields: | ||
assert(outfield in bg._outputs().traits()) | ||
|
||
# now try without defining outfields, we should get anat and func for free | ||
# now try without defining outfields | ||
bg = nio.BIDSDataGrabber() | ||
for outfield in ['anat', 'func']: | ||
for outfield in ['T1w', 'bold']: | ||
assert outfield in bg._outputs().traits() | ||
|
||
|
||
|
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.
This should be
mandatory=True
, since you don't use anisdefined()
check on it below. It's possible to pass anUndefined
to things with defaults to disable the input, unlessmandatory
is setTrue
.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.
ok!