From 5b02964c80e4dbd875cfa308ca565ab386d0d40b Mon Sep 17 00:00:00 2001 From: Carlos Correa Date: Sat, 4 Feb 2017 12:21:31 -0800 Subject: [PATCH 1/6] FSL FNIRT: handle multiple intensity map inputs and outputs --- nipype/interfaces/fsl/preprocess.py | 42 +++++++++++--- .../interfaces/fsl/tests/test_preprocess.py | 57 +++++++++++-------- 2 files changed, 67 insertions(+), 32 deletions(-) diff --git a/nipype/interfaces/fsl/preprocess.py b/nipype/interfaces/fsl/preprocess.py index 842bbbe978..4847da0c80 100644 --- a/nipype/interfaces/fsl/preprocess.py +++ b/nipype/interfaces/fsl/preprocess.py @@ -25,7 +25,7 @@ from ..base import (TraitedSpec, File, InputMultiPath, OutputMultiPath, Undefined, traits, isdefined) -from .base import FSLCommand, FSLCommandInputSpec +from .base import FSLCommand, FSLCommandInputSpec, Info class BETInputSpec(FSLCommandInputSpec): @@ -750,10 +750,12 @@ class FNIRTInputSpec(FSLCommandInputSpec): desc='name of file containing affine transform') inwarp_file = File(exists=True, argstr='--inwarp=%s', desc='name of file containing initial non-linear warps') - in_intensitymap_file = File(exists=True, argstr='--intin=%s', - desc=('name of file/files containing initial ' - 'intensity maping usually generated by ' - 'previous fnirt run')) + in_intensitymap_file = traits.List(File, exists=True, argstr='--intin=%s', + copyfiles=False, minlen=1, maxlen=2, + desc=('name of file/files containing ' + 'initial intensity mapping ' + 'usually generated by previous ' + 'fnirt run')) fieldcoeff_file = traits.Either( traits.Bool, File, argstr='--cout=%s', desc='name of output file with field coefficients or true') @@ -907,8 +909,9 @@ class FNIRTOutputSpec(TraitedSpec): field_file = File(desc='file with warp field') jacobian_file = File(desc='file containing Jacobian of the field') modulatedref_file = File(desc='file containing intensity modulated --ref') - out_intensitymap_file = File( - desc='file containing info pertaining to intensity mapping') + out_intensitymap_file = traits.List( + File, minlen=2, maxlen=2, + desc='files containing info pertaining to intensity mapping') log_file = File(desc='Name of log-file') @@ -975,9 +978,23 @@ def _list_outputs(self): change_ext=change_ext) else: outputs[key] = os.path.abspath(inval) + + if key == 'out_intensitymap_file' and isdefined(outputs[key]): + basename = FNIRT.intensitymap_file_basename(outputs[key]) + outputs[key] = [ + outputs[key], + '%s.txt' % basename, + ] return outputs def _format_arg(self, name, spec, value): + if name in ('in_intensitymap_file', 'out_intensitymap_file'): + if name == 'out_intensitymap_file': + value = self._list_outputs()[name] + value = map(FNIRT.intensitymap_file_basename, value) + assert len(set(value)) == 1, ( + 'Found different basenames for {}: {}'.format(name, value)) + return spec.argstr % value[0] if name in list(self.filemap.keys()): return spec.argstr % self._list_outputs()[name] return super(FNIRT, self)._format_arg(name, spec, value) @@ -1005,6 +1022,17 @@ def write_config(self, configfile): fid.write('%s\n' % (item)) fid.close() + @classmethod + def intensitymap_file_basename(cls, f): + """Removes valid intensitymap extensions from `f`, returning a basename + that can refer to both intensitymap files. + """ + for ext in Info.ftypes.values() + ['.txt']: + if f.endswith(ext): + return f[:-len(ext)] + # TODO consider warning for this case + return f + class ApplyWarpInputSpec(FSLCommandInputSpec): in_file = File(exists=True, argstr='--in=%s', diff --git a/nipype/interfaces/fsl/tests/test_preprocess.py b/nipype/interfaces/fsl/tests/test_preprocess.py index bcf52fb5f3..ce2d8f06f1 100644 --- a/nipype/interfaces/fsl/tests/test_preprocess.py +++ b/nipype/interfaces/fsl/tests/test_preprocess.py @@ -353,6 +353,7 @@ def test_mcflirt(setup_flirt): def test_fnirt(setup_flirt): tmpdir, infile, reffile = setup_flirt + os.chdir(tmpdir) fnirt = fsl.FNIRT() assert fnirt.cmd == 'fnirt' @@ -404,60 +405,66 @@ def test_fnirt(setup_flirt): fnirt.run() fnirt.inputs.in_file = infile fnirt.inputs.ref_file = reffile + infile_basename = fsl.FNIRT.intensitymap_file_basename(infile) # test files - opt_map = { - 'affine_file': ('--aff='), - 'inwarp_file': ('--inwarp='), - 'in_intensitymap_file': ('--intin='), - 'config_file': ('--config='), - 'refmask_file': ('--refmask='), - 'inmask_file': ('--inmask='), - 'field_file': ('--fout='), - 'jacobian_file': ('--jout='), - 'modulatedref_file': ('--refout='), - 'out_intensitymap_file': ('--intout='), - 'log_file': ('--logout=')} - - for name, settings in list(opt_map.items()): + opt_map = [ + ('affine_file', '--aff=%s' % infile, infile), + ('inwarp_file', '--inwarp=%s' % infile, infile), + ('in_intensitymap_file', '--intin=%s' % infile_basename, [infile]), + ('in_intensitymap_file', + '--intin=%s' % infile_basename, + [infile, '%s.txt' % infile_basename]), + ('config_file', '--config=%s' % infile, infile), + ('refmask_file', '--refmask=%s' % infile, infile), + ('inmask_file', '--inmask=%s' % infile, infile), + ('field_file', '--fout=%s' % infile, infile), + ('jacobian_file', '--jout=%s' % infile, infile), + ('modulatedref_file', '--refout=%s' % infile, infile), + ('out_intensitymap_file', + '--intout=%s_intmap' % infile_basename, True), + ('out_intensitymap_file', '--intout=%s' % infile_basename, infile), + ('log_file', '--logout=%s' % infile, infile)] + + for (name, settings, arg) in opt_map: fnirt = fsl.FNIRT(in_file=infile, ref_file=reffile, - **{name: infile}) + **{name: arg}) if name in ('config_file', 'affine_file', 'field_file'): - cmd = 'fnirt %s%s --in=%s '\ + cmd = 'fnirt %s --in=%s '\ '--logout=%s '\ - '--ref=%s --iout=%s' % (settings, infile, infile, log, + '--ref=%s --iout=%s' % (settings, infile, log, reffile, iout) elif name in ('refmask_file'): cmd = 'fnirt --in=%s '\ '--logout=%s --ref=%s '\ - '%s%s '\ + '%s '\ '--iout=%s' % (infile, log, reffile, - settings, infile, + settings, iout) elif name in ('in_intensitymap_file', 'inwarp_file', 'inmask_file', 'jacobian_file'): cmd = 'fnirt --in=%s '\ - '%s%s '\ + '%s '\ '--logout=%s --ref=%s '\ '--iout=%s' % (infile, - settings, infile, + settings, log, reffile, iout) elif name in ('log_file'): cmd = 'fnirt --in=%s '\ - '%s%s --ref=%s '\ + '%s --ref=%s '\ '--iout=%s' % (infile, - settings, infile, + settings, reffile, iout) else: cmd = 'fnirt --in=%s '\ - '--logout=%s %s%s '\ + '--logout=%s %s '\ '--ref=%s --iout=%s' % (infile, log, - settings, infile, + settings, reffile, iout) assert fnirt.cmdline == cmd From b2888e09db74822f56ea984e063b7f81c3842527 Mon Sep 17 00:00:00 2001 From: Carlos Correa Date: Sat, 4 Feb 2017 12:22:11 -0800 Subject: [PATCH 2/6] FSL FNIRT: add test for fieldcoeff_file input --- nipype/interfaces/fsl/tests/test_preprocess.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/nipype/interfaces/fsl/tests/test_preprocess.py b/nipype/interfaces/fsl/tests/test_preprocess.py index ce2d8f06f1..c2b8ab8597 100644 --- a/nipype/interfaces/fsl/tests/test_preprocess.py +++ b/nipype/interfaces/fsl/tests/test_preprocess.py @@ -424,6 +424,7 @@ def test_fnirt(setup_flirt): ('out_intensitymap_file', '--intout=%s_intmap' % infile_basename, True), ('out_intensitymap_file', '--intout=%s' % infile_basename, infile), + ('fieldcoeff_file', '--cout=%s' % infile, infile), ('log_file', '--logout=%s' % infile, infile)] for (name, settings, arg) in opt_map: @@ -431,7 +432,7 @@ def test_fnirt(setup_flirt): ref_file=reffile, **{name: arg}) - if name in ('config_file', 'affine_file', 'field_file'): + if name in ('config_file', 'affine_file', 'field_file', 'fieldcoeff_file'): cmd = 'fnirt %s --in=%s '\ '--logout=%s '\ '--ref=%s --iout=%s' % (settings, infile, log, From fa5cde82aa5f1df70e6cf1f93bf010ecfee3ec7f Mon Sep 17 00:00:00 2001 From: Carlos Correa Date: Sun, 5 Feb 2017 16:26:08 -0800 Subject: [PATCH 3/6] Address PR comment by making sure exists=True is kwarg for File --- nipype/interfaces/fsl/preprocess.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nipype/interfaces/fsl/preprocess.py b/nipype/interfaces/fsl/preprocess.py index 4847da0c80..63814f1bc9 100644 --- a/nipype/interfaces/fsl/preprocess.py +++ b/nipype/interfaces/fsl/preprocess.py @@ -750,7 +750,7 @@ class FNIRTInputSpec(FSLCommandInputSpec): desc='name of file containing affine transform') inwarp_file = File(exists=True, argstr='--inwarp=%s', desc='name of file containing initial non-linear warps') - in_intensitymap_file = traits.List(File, exists=True, argstr='--intin=%s', + in_intensitymap_file = traits.List(File(exists=True), argstr='--intin=%s', copyfiles=False, minlen=1, maxlen=2, desc=('name of file/files containing ' 'initial intensity mapping ' From 69e178c518639464cceadd64f7a350136711a671 Mon Sep 17 00:00:00 2001 From: Carlos Correa Date: Tue, 7 Feb 2017 12:35:03 -0800 Subject: [PATCH 4/6] FSL FNIRT: ensure txt intensity map file exists --- nipype/interfaces/fsl/tests/test_preprocess.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/nipype/interfaces/fsl/tests/test_preprocess.py b/nipype/interfaces/fsl/tests/test_preprocess.py index c2b8ab8597..bd4b69022c 100644 --- a/nipype/interfaces/fsl/tests/test_preprocess.py +++ b/nipype/interfaces/fsl/tests/test_preprocess.py @@ -406,6 +406,10 @@ def test_fnirt(setup_flirt): fnirt.inputs.in_file = infile fnirt.inputs.ref_file = reffile infile_basename = fsl.FNIRT.intensitymap_file_basename(infile) + infile_txt = '%s.txt' % infile_basename + # doing this to create the file to pass tests for file existence + with open(infile_txt, 'w'): + pass # test files opt_map = [ @@ -414,7 +418,7 @@ def test_fnirt(setup_flirt): ('in_intensitymap_file', '--intin=%s' % infile_basename, [infile]), ('in_intensitymap_file', '--intin=%s' % infile_basename, - [infile, '%s.txt' % infile_basename]), + [infile, infile_txt]), ('config_file', '--config=%s' % infile, infile), ('refmask_file', '--refmask=%s' % infile, infile), ('inmask_file', '--inmask=%s' % infile, infile), From 1a4db10fbf91ccddf277d84e830997f118e65955 Mon Sep 17 00:00:00 2001 From: Carlos Correa Date: Tue, 7 Feb 2017 12:50:58 -0800 Subject: [PATCH 5/6] FSL FNIRT: check list_outputs to increase code coverage --- .../interfaces/fsl/tests/test_preprocess.py | 22 ++++++++++++------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/nipype/interfaces/fsl/tests/test_preprocess.py b/nipype/interfaces/fsl/tests/test_preprocess.py index bd4b69022c..f9242c4b26 100644 --- a/nipype/interfaces/fsl/tests/test_preprocess.py +++ b/nipype/interfaces/fsl/tests/test_preprocess.py @@ -405,20 +405,23 @@ def test_fnirt(setup_flirt): fnirt.run() fnirt.inputs.in_file = infile fnirt.inputs.ref_file = reffile - infile_basename = fsl.FNIRT.intensitymap_file_basename(infile) - infile_txt = '%s.txt' % infile_basename + intmap_basename = '%s_intmap' % fsl.FNIRT.intensitymap_file_basename(infile) + intmap_image = fsl_name(fnirt, intmap_basename) + intmap_txt = '%s.txt' % intmap_basename # doing this to create the file to pass tests for file existence - with open(infile_txt, 'w'): + with open(intmap_image, 'w'): + pass + with open(intmap_txt, 'w'): pass # test files opt_map = [ ('affine_file', '--aff=%s' % infile, infile), ('inwarp_file', '--inwarp=%s' % infile, infile), - ('in_intensitymap_file', '--intin=%s' % infile_basename, [infile]), + ('in_intensitymap_file', '--intin=%s' % intmap_basename, [intmap_image]), ('in_intensitymap_file', - '--intin=%s' % infile_basename, - [infile, infile_txt]), + '--intin=%s' % intmap_basename, + [intmap_image, intmap_txt]), ('config_file', '--config=%s' % infile, infile), ('refmask_file', '--refmask=%s' % infile, infile), ('inmask_file', '--inmask=%s' % infile, infile), @@ -426,8 +429,8 @@ def test_fnirt(setup_flirt): ('jacobian_file', '--jout=%s' % infile, infile), ('modulatedref_file', '--refout=%s' % infile, infile), ('out_intensitymap_file', - '--intout=%s_intmap' % infile_basename, True), - ('out_intensitymap_file', '--intout=%s' % infile_basename, infile), + '--intout=%s' % intmap_basename, True), + ('out_intensitymap_file', '--intout=%s' % intmap_basename, intmap_image), ('fieldcoeff_file', '--cout=%s' % infile, infile), ('log_file', '--logout=%s' % infile, infile)] @@ -474,6 +477,9 @@ def test_fnirt(setup_flirt): assert fnirt.cmdline == cmd + if name == 'out_intensitymap_file': + assert fnirt._list_outputs()['out_intensitymap_file'] == [ + intmap_image, intmap_txt] @pytest.mark.skipif(no_fsl(), reason="fsl is not installed") def test_applywarp(setup_flirt): From 7a8eb0714109b1495035a45add295427bcd15278 Mon Sep 17 00:00:00 2001 From: Carlos Correa Date: Tue, 7 Feb 2017 12:53:06 -0800 Subject: [PATCH 6/6] FSL FNIRT: fix python3 issue with dict values and map --- nipype/interfaces/fsl/preprocess.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/nipype/interfaces/fsl/preprocess.py b/nipype/interfaces/fsl/preprocess.py index 63814f1bc9..eaab2a830e 100644 --- a/nipype/interfaces/fsl/preprocess.py +++ b/nipype/interfaces/fsl/preprocess.py @@ -991,7 +991,7 @@ def _format_arg(self, name, spec, value): if name in ('in_intensitymap_file', 'out_intensitymap_file'): if name == 'out_intensitymap_file': value = self._list_outputs()[name] - value = map(FNIRT.intensitymap_file_basename, value) + value = [FNIRT.intensitymap_file_basename(v) for v in value] assert len(set(value)) == 1, ( 'Found different basenames for {}: {}'.format(name, value)) return spec.argstr % value[0] @@ -1027,7 +1027,7 @@ def intensitymap_file_basename(cls, f): """Removes valid intensitymap extensions from `f`, returning a basename that can refer to both intensitymap files. """ - for ext in Info.ftypes.values() + ['.txt']: + for ext in list(Info.ftypes.values()) + ['.txt']: if f.endswith(ext): return f[:-len(ext)] # TODO consider warning for this case