From bf61ba6b2e4654496e72265097fcc9f5e3a31f3e Mon Sep 17 00:00:00 2001 From: mathiasg Date: Thu, 9 Feb 2017 21:43:49 -0500 Subject: [PATCH 01/13] fix: decode instead of ignore --- nipype/interfaces/mrtrix/convert.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/nipype/interfaces/mrtrix/convert.py b/nipype/interfaces/mrtrix/convert.py index e76e847535..348fc1c898 100644 --- a/nipype/interfaces/mrtrix/convert.py +++ b/nipype/interfaces/mrtrix/convert.py @@ -10,7 +10,7 @@ """ from __future__ import print_function, division, unicode_literals, absolute_import -from builtins import open +from io import open import os.path as op import nibabel as nb @@ -55,10 +55,11 @@ def read_mrtrix_tracks(in_file, as_generator=True): def read_mrtrix_header(in_file): - fileobj = open(in_file, 'r') + fileobj = open(in_file, 'rb') header = {} iflogger.info('Reading header data...') for line in fileobj: + line = line.decode() if line == 'END\n': iflogger.info('Reached the end of the header!') break @@ -78,7 +79,7 @@ def read_mrtrix_header(in_file): def read_mrtrix_streamlines(in_file, header, as_generator=True): offset = header['offset'] stream_count = header['count'] - fileobj = open(in_file, 'r') + fileobj = open(in_file, 'rb') fileobj.seek(offset) endianness = native_code f4dt = np.dtype(endianness + 'f4') @@ -90,7 +91,7 @@ def points_per_track(offset): n_points = 0 track_points = [] iflogger.info('Identifying the number of points per tract...') - all_str = fileobj.read() + all_str = fileobj.read().decode() num_triplets = int(len(all_str) / bytesize) pts = np.ndarray(shape=(num_triplets, pt_cols), dtype='f4', buffer=all_str) nonfinite_list = np.where(np.isfinite(pts[:, 2]) == False) From 18eedf70004e42772a39dd57790728cbf79c7324 Mon Sep 17 00:00:00 2001 From: mathiasg Date: Fri, 10 Feb 2017 11:48:54 -0500 Subject: [PATCH 02/13] fix: make outvol mandatory --- nipype/interfaces/freesurfer/preprocess.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nipype/interfaces/freesurfer/preprocess.py b/nipype/interfaces/freesurfer/preprocess.py index 2953963149..d5d40e88af 100644 --- a/nipype/interfaces/freesurfer/preprocess.py +++ b/nipype/interfaces/freesurfer/preprocess.py @@ -1467,7 +1467,7 @@ class MNIBiasCorrectionInputSpec(FSTraitedSpec): # mandatory in_file = File(exists=True, mandatory=True, argstr="--i %s", desc="input volume. Input can be any format accepted by mri_convert.") - out_file = File(argstr="--o %s", name_source=['in_file'], + out_file = File(argstr="--o %s", mandatory=True, name_source=['in_file'], name_template='%s_output', hash_files=False, keep_extension=True, desc="output volume. Output can be any format accepted by mri_convert. " + "If the output format is COR, then the directory must exist.") From 2cecd53ca6c959c41c9ecd006bd2e1c18106c1b7 Mon Sep 17 00:00:00 2001 From: mathiasg Date: Fri, 10 Feb 2017 17:55:18 -0500 Subject: [PATCH 03/13] enh: testing --- .../freesurfer/tests/test_preprocess.py | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/nipype/interfaces/freesurfer/tests/test_preprocess.py b/nipype/interfaces/freesurfer/tests/test_preprocess.py index da6ba65fc3..f4a468bb52 100644 --- a/nipype/interfaces/freesurfer/tests/test_preprocess.py +++ b/nipype/interfaces/freesurfer/tests/test_preprocess.py @@ -85,3 +85,28 @@ def test_synthesizeflash(create_files_in_directory): syn2 = freesurfer.SynthesizeFLASH(t1_image=filelist[0], pd_image=filelist[1], flip_angle=20, te=5, tr=25) assert syn2.cmdline == ('mri_synthesize 25.00 20.00 5.000 %s %s %s' % (filelist[0], filelist[1], os.path.join(outdir, 'synth-flash_20.mgz'))) + +@pytest.mark.skipif(freesurfer.no_freesurfer(), reason="freesurfer is not installed") +def test_mandatory_outvol(create_files_in_directory): + filelist, outdir = create_files_in_directory + mni = freesurfer.MNIBiasCorrection() + + # make sure command gets called + assert mni.cmd == "mri_nu_correct.mni" + + # test raising error with mandatory args absent + with pytest.raises(ValueError): mni.run() + + # test raising error with only partial mandatory args present + mni.inputs.in_file = filelist[0] #mgz + with pytest.raises(ValueError): mni.run() + + # rest of mandatory inputs + mni.inputs.out_file = 'bias_corrected_file' + + assert mni.cmdline == ('mri_nu_correct.mni --i %s --n 4 --o bias_corrected_file.mgz' + % filelist[0]) + # constructor based tests + mni2 = freesurfer.MNIBiasCorrection(in_file=filelist[0], out_file='bias_corrected_file') + assert mni2.cmdline == ('mri_nu_correct.mni --i %s --n 4 --o bias_corrected_file.mgz' + % filelist[0]) From 43182745b0fdab212b6ec5613d08b1273ea7a590 Mon Sep 17 00:00:00 2001 From: mathiasg Date: Fri, 10 Feb 2017 22:32:48 -0500 Subject: [PATCH 04/13] fix: doctest --- nipype/interfaces/freesurfer/preprocess.py | 1 + nipype/interfaces/freesurfer/tests/test_preprocess.py | 8 ++++---- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/nipype/interfaces/freesurfer/preprocess.py b/nipype/interfaces/freesurfer/preprocess.py index d5d40e88af..322b0a02c2 100644 --- a/nipype/interfaces/freesurfer/preprocess.py +++ b/nipype/interfaces/freesurfer/preprocess.py @@ -1510,6 +1510,7 @@ class MNIBiasCorrection(FSCommand): >>> from nipype.interfaces.freesurfer import MNIBiasCorrection >>> correct = MNIBiasCorrection() >>> correct.inputs.in_file = "norm.mgz" + >>> correct.inputs.out_file = "norm_output.mgz" >>> correct.inputs.iterations = 6 >>> correct.inputs.protocol_iterations = 1000 >>> correct.inputs.distance = 50 diff --git a/nipype/interfaces/freesurfer/tests/test_preprocess.py b/nipype/interfaces/freesurfer/tests/test_preprocess.py index f4a468bb52..d0cc9c850b 100644 --- a/nipype/interfaces/freesurfer/tests/test_preprocess.py +++ b/nipype/interfaces/freesurfer/tests/test_preprocess.py @@ -102,11 +102,11 @@ def test_mandatory_outvol(create_files_in_directory): with pytest.raises(ValueError): mni.run() # rest of mandatory inputs - mni.inputs.out_file = 'bias_corrected_file' + mni.inputs.out_file = 'bias_corrected_output' - assert mni.cmdline == ('mri_nu_correct.mni --i %s --n 4 --o bias_corrected_file.mgz' + assert mni.cmdline == ('mri_nu_correct.mni --i %s --n 4 --o bias_corrected_output.mgz' % filelist[0]) # constructor based tests - mni2 = freesurfer.MNIBiasCorrection(in_file=filelist[0], out_file='bias_corrected_file') - assert mni2.cmdline == ('mri_nu_correct.mni --i %s --n 4 --o bias_corrected_file.mgz' + mni2 = freesurfer.MNIBiasCorrection(in_file=filelist[0], out_file='bias_corrected_output') + assert mni2.cmdline == ('mri_nu_correct.mni --i %s --n 4 --o bias_corrected_output.mgz' % filelist[0]) From 57826345dc552641c16730856090502657cffd8f Mon Sep 17 00:00:00 2001 From: mathiasg Date: Tue, 14 Feb 2017 13:03:14 -0500 Subject: [PATCH 05/13] enh: genfile --- nipype/interfaces/freesurfer/preprocess.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/nipype/interfaces/freesurfer/preprocess.py b/nipype/interfaces/freesurfer/preprocess.py index 322b0a02c2..4b3851d689 100644 --- a/nipype/interfaces/freesurfer/preprocess.py +++ b/nipype/interfaces/freesurfer/preprocess.py @@ -1467,11 +1467,11 @@ class MNIBiasCorrectionInputSpec(FSTraitedSpec): # mandatory in_file = File(exists=True, mandatory=True, argstr="--i %s", desc="input volume. Input can be any format accepted by mri_convert.") - out_file = File(argstr="--o %s", mandatory=True, name_source=['in_file'], + # optional + out_file = File(argstr="--o %s", name_source=['in_file'], genfile=True, name_template='%s_output', hash_files=False, keep_extension=True, desc="output volume. Output can be any format accepted by mri_convert. " + "If the output format is COR, then the directory must exist.") - # optional iterations = traits.Int(4, argstr="--n %d", desc="Number of iterations to run nu_correct. Default is 4. This is the number of times " + "that nu_correct is repeated (ie, using the output from the previous run as the input for " + @@ -1510,7 +1510,6 @@ class MNIBiasCorrection(FSCommand): >>> from nipype.interfaces.freesurfer import MNIBiasCorrection >>> correct = MNIBiasCorrection() >>> correct.inputs.in_file = "norm.mgz" - >>> correct.inputs.out_file = "norm_output.mgz" >>> correct.inputs.iterations = 6 >>> correct.inputs.protocol_iterations = 1000 >>> correct.inputs.distance = 50 From fd1dda6d14b2d747911b88b85043be92e4256141 Mon Sep 17 00:00:00 2001 From: mathiasg Date: Tue, 14 Feb 2017 13:23:25 -0500 Subject: [PATCH 06/13] fix: outputspec --- nipype/interfaces/freesurfer/preprocess.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/nipype/interfaces/freesurfer/preprocess.py b/nipype/interfaces/freesurfer/preprocess.py index 4b3851d689..3ce1423c22 100644 --- a/nipype/interfaces/freesurfer/preprocess.py +++ b/nipype/interfaces/freesurfer/preprocess.py @@ -1468,7 +1468,7 @@ class MNIBiasCorrectionInputSpec(FSTraitedSpec): in_file = File(exists=True, mandatory=True, argstr="--i %s", desc="input volume. Input can be any format accepted by mri_convert.") # optional - out_file = File(argstr="--o %s", name_source=['in_file'], genfile=True, + out_file = File(argstr="--o %s", name_source=['in_file'], name_template='%s_output', hash_files=False, keep_extension=True, desc="output volume. Output can be any format accepted by mri_convert. " + "If the output format is COR, then the directory must exist.") @@ -1528,7 +1528,7 @@ class MNIBiasCorrection(FSCommand): output_spec = MNIBiasCorrectionOutputSpec def _list_outputs(self): - outputs = self._outputs().get() + outputs = self._output_spec().get() outputs["out_file"] = os.path.abspath(self.inputs.out_file) return outputs From cb74279143d48627718e7c08125540bedc979821 Mon Sep 17 00:00:00 2001 From: mathiasg Date: Tue, 14 Feb 2017 13:30:47 -0500 Subject: [PATCH 07/13] fix: outputspec attribute --- nipype/interfaces/freesurfer/preprocess.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nipype/interfaces/freesurfer/preprocess.py b/nipype/interfaces/freesurfer/preprocess.py index 6d517427a2..882aebf88d 100644 --- a/nipype/interfaces/freesurfer/preprocess.py +++ b/nipype/interfaces/freesurfer/preprocess.py @@ -1537,7 +1537,7 @@ class MNIBiasCorrection(FSCommand): output_spec = MNIBiasCorrectionOutputSpec def _list_outputs(self): - outputs = self._output_spec().get() + outputs = self.output_spec().get() outputs["out_file"] = os.path.abspath(self.inputs.out_file) return outputs From a5fc44024fbbdb93c459eddbb36520525ebbf2c8 Mon Sep 17 00:00:00 2001 From: mathiasg Date: Tue, 14 Feb 2017 16:07:16 -0500 Subject: [PATCH 08/13] fix: grab out file --- nipype/interfaces/freesurfer/preprocess.py | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/nipype/interfaces/freesurfer/preprocess.py b/nipype/interfaces/freesurfer/preprocess.py index 882aebf88d..0566a67647 100644 --- a/nipype/interfaces/freesurfer/preprocess.py +++ b/nipype/interfaces/freesurfer/preprocess.py @@ -1477,7 +1477,7 @@ class MNIBiasCorrectionInputSpec(FSTraitedSpec): in_file = File(exists=True, mandatory=True, argstr="--i %s", desc="input volume. Input can be any format accepted by mri_convert.") # optional - out_file = File(argstr="--o %s", name_source=['in_file'], + out_file = File(argstr="--o %s", name_source=['in_file'], genfile=True, name_template='%s_output', hash_files=False, keep_extension=True, desc="output volume. Output can be any format accepted by mri_convert. " + "If the output format is COR, then the directory must exist.") @@ -1501,7 +1501,7 @@ class MNIBiasCorrectionInputSpec(FSTraitedSpec): desc="Shrink parameter for finer sampling (default is 4)") class MNIBiasCorrectionOutputSpec(TraitedSpec): - out_file = File(desc="output volume") + out_file = File(exists=True, desc="output volume") class MNIBiasCorrection(FSCommand): @@ -1536,9 +1536,17 @@ class MNIBiasCorrection(FSCommand): input_spec = MNIBiasCorrectionInputSpec output_spec = MNIBiasCorrectionOutputSpec + def _gen_filename(self): + # if outfile was not defined + if not isdefined(self.inputs.out_file): + return self._gen_fname(self.inputs.in_file, + suffix='_output') def _list_outputs(self): outputs = self.output_spec().get() - outputs["out_file"] = os.path.abspath(self.inputs.out_file) + if not isdefined(self.inputs.out_file): + outputs["out_file"] = self._gen_filename() + else: + outputs["out_file"] = os.path.abspath(self.inputs.out_file) return outputs From 6a44c6e2aba3804370ee45e12ab7f0111489ea14 Mon Sep 17 00:00:00 2001 From: mathiasg Date: Tue, 14 Feb 2017 16:29:26 -0500 Subject: [PATCH 09/13] fix: mnibias test --- .../freesurfer/tests/test_preprocess.py | 21 +++++++++++-------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/nipype/interfaces/freesurfer/tests/test_preprocess.py b/nipype/interfaces/freesurfer/tests/test_preprocess.py index d0cc9c850b..fbca79a22e 100644 --- a/nipype/interfaces/freesurfer/tests/test_preprocess.py +++ b/nipype/interfaces/freesurfer/tests/test_preprocess.py @@ -95,18 +95,21 @@ def test_mandatory_outvol(create_files_in_directory): assert mni.cmd == "mri_nu_correct.mni" # test raising error with mandatory args absent - with pytest.raises(ValueError): mni.run() + with pytest.raises(ValueError): mni.cmdline - # test raising error with only partial mandatory args present - mni.inputs.in_file = filelist[0] #mgz - with pytest.raises(ValueError): mni.run() + # test with minimal args + mni.inputs.in_file = filelist[0] + assert mni.cmdline == ('mri_nu_correct.mni --i %s --o %s_output.mgz' + % (filelist[0], filelist[0].replace('.mgz', '')) - # rest of mandatory inputs - mni.inputs.out_file = 'bias_corrected_output' + # test with custom outfile + mni.inputs.out_file = 'new_corrected_file.mgz' + assert mni.cmdline == ('mri_nu_correct.mni --i %s --o new_corrected_file.mgz' + % (filelist[0]) - assert mni.cmdline == ('mri_nu_correct.mni --i %s --n 4 --o bias_corrected_output.mgz' - % filelist[0]) # constructor based tests - mni2 = freesurfer.MNIBiasCorrection(in_file=filelist[0], out_file='bias_corrected_output') + mni2 = freesurfer.MNIBiasCorrection(in_file=filelist[0], + out_file='bias_corrected_output', + iterations=4) assert mni2.cmdline == ('mri_nu_correct.mni --i %s --n 4 --o bias_corrected_output.mgz' % filelist[0]) From b97a329b43bfcb6469707cbdec6c69026b5459fd Mon Sep 17 00:00:00 2001 From: mathiasg Date: Tue, 14 Feb 2017 16:31:57 -0500 Subject: [PATCH 10/13] fix: removed changed file --- nipype/interfaces/mrtrix/convert.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/nipype/interfaces/mrtrix/convert.py b/nipype/interfaces/mrtrix/convert.py index 348fc1c898..e76e847535 100644 --- a/nipype/interfaces/mrtrix/convert.py +++ b/nipype/interfaces/mrtrix/convert.py @@ -10,7 +10,7 @@ """ from __future__ import print_function, division, unicode_literals, absolute_import -from io import open +from builtins import open import os.path as op import nibabel as nb @@ -55,11 +55,10 @@ def read_mrtrix_tracks(in_file, as_generator=True): def read_mrtrix_header(in_file): - fileobj = open(in_file, 'rb') + fileobj = open(in_file, 'r') header = {} iflogger.info('Reading header data...') for line in fileobj: - line = line.decode() if line == 'END\n': iflogger.info('Reached the end of the header!') break @@ -79,7 +78,7 @@ def read_mrtrix_header(in_file): def read_mrtrix_streamlines(in_file, header, as_generator=True): offset = header['offset'] stream_count = header['count'] - fileobj = open(in_file, 'rb') + fileobj = open(in_file, 'r') fileobj.seek(offset) endianness = native_code f4dt = np.dtype(endianness + 'f4') @@ -91,7 +90,7 @@ def points_per_track(offset): n_points = 0 track_points = [] iflogger.info('Identifying the number of points per tract...') - all_str = fileobj.read().decode() + all_str = fileobj.read() num_triplets = int(len(all_str) / bytesize) pts = np.ndarray(shape=(num_triplets, pt_cols), dtype='f4', buffer=all_str) nonfinite_list = np.where(np.isfinite(pts[:, 2]) == False) From 8231470cd4cb7205e6a8563f28ed3558af154120 Mon Sep 17 00:00:00 2001 From: mathiasg Date: Wed, 15 Feb 2017 10:44:17 -0500 Subject: [PATCH 11/13] fix: use _outputs() --- nipype/interfaces/freesurfer/preprocess.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nipype/interfaces/freesurfer/preprocess.py b/nipype/interfaces/freesurfer/preprocess.py index 0566a67647..d616c622ee 100644 --- a/nipype/interfaces/freesurfer/preprocess.py +++ b/nipype/interfaces/freesurfer/preprocess.py @@ -1542,7 +1542,7 @@ def _gen_filename(self): return self._gen_fname(self.inputs.in_file, suffix='_output') def _list_outputs(self): - outputs = self.output_spec().get() + outputs = self._outputs().get() if not isdefined(self.inputs.out_file): outputs["out_file"] = self._gen_filename() else: From 8d935fb4205c354a383af556a83122c442ace3aa Mon Sep 17 00:00:00 2001 From: mathiasg Date: Wed, 15 Feb 2017 13:10:17 -0500 Subject: [PATCH 12/13] fix: removed list outputs and test changes --- nipype/interfaces/freesurfer/preprocess.py | 13 ------------- .../interfaces/freesurfer/tests/test_preprocess.py | 4 ++-- 2 files changed, 2 insertions(+), 15 deletions(-) diff --git a/nipype/interfaces/freesurfer/preprocess.py b/nipype/interfaces/freesurfer/preprocess.py index d616c622ee..3ebc26f23a 100644 --- a/nipype/interfaces/freesurfer/preprocess.py +++ b/nipype/interfaces/freesurfer/preprocess.py @@ -1536,19 +1536,6 @@ class MNIBiasCorrection(FSCommand): input_spec = MNIBiasCorrectionInputSpec output_spec = MNIBiasCorrectionOutputSpec - def _gen_filename(self): - # if outfile was not defined - if not isdefined(self.inputs.out_file): - return self._gen_fname(self.inputs.in_file, - suffix='_output') - def _list_outputs(self): - outputs = self._outputs().get() - if not isdefined(self.inputs.out_file): - outputs["out_file"] = self._gen_filename() - else: - outputs["out_file"] = os.path.abspath(self.inputs.out_file) - return outputs - class WatershedSkullStripInputSpec(FSTraitedSpec): # required diff --git a/nipype/interfaces/freesurfer/tests/test_preprocess.py b/nipype/interfaces/freesurfer/tests/test_preprocess.py index fbca79a22e..60a51e3a86 100644 --- a/nipype/interfaces/freesurfer/tests/test_preprocess.py +++ b/nipype/interfaces/freesurfer/tests/test_preprocess.py @@ -100,12 +100,12 @@ def test_mandatory_outvol(create_files_in_directory): # test with minimal args mni.inputs.in_file = filelist[0] assert mni.cmdline == ('mri_nu_correct.mni --i %s --o %s_output.mgz' - % (filelist[0], filelist[0].replace('.mgz', '')) + % (filelist[0], filelist[0].replace('.mgz', ''))) # test with custom outfile mni.inputs.out_file = 'new_corrected_file.mgz' assert mni.cmdline == ('mri_nu_correct.mni --i %s --o new_corrected_file.mgz' - % (filelist[0]) + % (filelist[0])) # constructor based tests mni2 = freesurfer.MNIBiasCorrection(in_file=filelist[0], From ba1a86319d0cb1f8ef6f86dc70f61decbce2a428 Mon Sep 17 00:00:00 2001 From: mathiasg Date: Thu, 16 Feb 2017 14:22:12 -0500 Subject: [PATCH 13/13] removed genfile --- nipype/interfaces/freesurfer/preprocess.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nipype/interfaces/freesurfer/preprocess.py b/nipype/interfaces/freesurfer/preprocess.py index 4be3185644..c474d7616d 100644 --- a/nipype/interfaces/freesurfer/preprocess.py +++ b/nipype/interfaces/freesurfer/preprocess.py @@ -1477,7 +1477,7 @@ class MNIBiasCorrectionInputSpec(FSTraitedSpec): in_file = File(exists=True, mandatory=True, argstr="--i %s", desc="input volume. Input can be any format accepted by mri_convert.") # optional - out_file = File(argstr="--o %s", name_source=['in_file'], genfile=True, + out_file = File(argstr="--o %s", name_source=['in_file'], name_template='%s_output', hash_files=False, keep_extension=True, desc="output volume. Output can be any format accepted by mri_convert. " + "If the output format is COR, then the directory must exist.")