From 0430c8565a041465c9efa74c5d089ef1ee0b4009 Mon Sep 17 00:00:00 2001 From: Yaroslav Halchenko Date: Fri, 11 Jan 2019 15:51:04 -0500 Subject: [PATCH 1/6] RF+ENH(TST): use %s instead of %f when rendering cmdline float point args This way makes it easier to test cmdline formation so we have 4.9 and not 4.90000 --- nipype/interfaces/ants/segmentation.py | 10 ++--- .../ants/tests/test_segmentation.py | 40 +++++++++++++++++++ 2 files changed, 45 insertions(+), 5 deletions(-) create mode 100644 nipype/interfaces/ants/tests/test_segmentation.py diff --git a/nipype/interfaces/ants/segmentation.py b/nipype/interfaces/ants/segmentation.py index 11bc0c48c7..94e8c59c79 100644 --- a/nipype/interfaces/ants/segmentation.py +++ b/nipype/interfaces/ants/segmentation.py @@ -208,24 +208,24 @@ class LaplacianThicknessInputSpec(ANTSCommandInputSpec): keep_extension=True, hash_files=False) smooth_param = traits.Float( - argstr='%f', + argstr='%s', desc='Sigma of the Laplacian Recursive Image Filter (defaults to 1)', position=4) prior_thickness = traits.Float( - argstr='%f', + argstr='%s', desc='Prior thickness (defaults to 500)', position=5) dT = traits.Float( - argstr='%f', + argstr='%s', desc='Time delta used during integration (defaults to 0.01)', position=6) sulcus_prior = traits.Float( - argstr='%f', + argstr='%s', desc='Positive floating point number for sulcus prior. ' 'Authors said that 0.15 might be a reasonable value', position=7) tolerance = traits.Float( - argstr='%f', + argstr='%s', desc='Tolerance to reach during optimization (defaults to 0.001)', position=8) diff --git a/nipype/interfaces/ants/tests/test_segmentation.py b/nipype/interfaces/ants/tests/test_segmentation.py new file mode 100644 index 0000000000..dca2ee68d4 --- /dev/null +++ b/nipype/interfaces/ants/tests/test_segmentation.py @@ -0,0 +1,40 @@ +# emacs: -*- mode: python; py-indent-offset: 4; indent-tabs-mode: nil -*- +# vi: set ft=python sts=4 ts=4 sw=4 et: + +from ..segmentation import LaplacianThickness +from .test_resampling import change_dir + +import os +import pytest + + +@pytest.fixture() +def change_dir(request): + orig_dir = os.getcwd() + filepath = os.path.dirname(os.path.realpath(__file__)) + datadir = os.path.realpath(os.path.join(filepath, '../../../testing/data')) + os.chdir(datadir) + + def move2orig(): + os.chdir(orig_dir) + + request.addfinalizer(move2orig) + + +@pytest.fixture() +def create_lt(): + lt = LaplacianThickness() + # we do not run, so I stick some not really proper files as input + lt.inputs.input_gm = 'diffusion_weighted.nii' + lt.inputs.input_wm = 'functional.nii' + return lt + + +def test_LaplacianThickness_defaults(change_dir, create_lt): + lt = create_lt + base_cmd = 'LaplacianThickness functional.nii diffusion_weighted.nii functional_thickness.nii' + assert lt.cmdline == base_cmd + lt.inputs.smooth_param = 4.5 + assert lt.cmdline == base_cmd + " 4.5" + lt.inputs.prior_thickness = 5.9 + assert lt.cmdline == base_cmd + " 4.5 5.9" From fc18c784266c53e831c0c4247a978613fd9f2985 Mon Sep 17 00:00:00 2001 From: Yaroslav Halchenko Date: Fri, 11 Jan 2019 15:53:27 -0500 Subject: [PATCH 2/6] TST(BK): test_LaplacianThickness_wrongargs to demonstrate #2847 --- nipype/interfaces/ants/tests/test_segmentation.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/nipype/interfaces/ants/tests/test_segmentation.py b/nipype/interfaces/ants/tests/test_segmentation.py index dca2ee68d4..d595317713 100644 --- a/nipype/interfaces/ants/tests/test_segmentation.py +++ b/nipype/interfaces/ants/tests/test_segmentation.py @@ -38,3 +38,12 @@ def test_LaplacianThickness_defaults(change_dir, create_lt): assert lt.cmdline == base_cmd + " 4.5" lt.inputs.prior_thickness = 5.9 assert lt.cmdline == base_cmd + " 4.5 5.9" + + +def test_LaplacianThickness_wrongargs(change_dir, create_lt): + lt = create_lt + lt.inputs.prior_thickness = 5.9 + # 500 must not be placed as smooth_param + assert lt.cmdline != 'LaplacianThickness functional.nii diffusion_weighted.nii functional_thickness.nii 5.9' + # probably should have just raised an exception that "smooth_param" + # should also be defined \ No newline at end of file From d633a57ce001452e420341616a608563911de7bd Mon Sep 17 00:00:00 2001 From: Yaroslav Halchenko Date: Thu, 24 Jan 2019 22:44:43 -0500 Subject: [PATCH 3/6] BF: provide chain of requires for LaplacianThickness Thanks @effigies. This should prevent incorrect specification` of parameters in the command line since for ants they are just positional ones, so all previous ones should be specified --- nipype/interfaces/ants/segmentation.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/nipype/interfaces/ants/segmentation.py b/nipype/interfaces/ants/segmentation.py index 94e8c59c79..adaf765527 100644 --- a/nipype/interfaces/ants/segmentation.py +++ b/nipype/interfaces/ants/segmentation.py @@ -214,19 +214,23 @@ class LaplacianThicknessInputSpec(ANTSCommandInputSpec): prior_thickness = traits.Float( argstr='%s', desc='Prior thickness (defaults to 500)', + requires=['smooth_param'], position=5) dT = traits.Float( argstr='%s', desc='Time delta used during integration (defaults to 0.01)', + requires=['prior_thickness'], position=6) sulcus_prior = traits.Float( argstr='%s', desc='Positive floating point number for sulcus prior. ' 'Authors said that 0.15 might be a reasonable value', + requires=['dT'], position=7) tolerance = traits.Float( argstr='%s', desc='Tolerance to reach during optimization (defaults to 0.001)', + requires=['sulcus_prior'], position=8) From 56b222751df496fca1f7dbdec20501f480a250d7 Mon Sep 17 00:00:00 2001 From: "Christopher J. Markiewicz" Date: Thu, 14 Feb 2019 10:31:06 -0500 Subject: [PATCH 4/6] FIX: Requires error text was backwards --- nipype/interfaces/base/core.py | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/nipype/interfaces/base/core.py b/nipype/interfaces/base/core.py index ae002cf17f..77ab6cf398 100644 --- a/nipype/interfaces/base/core.py +++ b/nipype/interfaces/base/core.py @@ -202,10 +202,16 @@ def _check_requires(self, spec, name, value): for field in spec.requires ] if any(values) and isdefined(value): - msg = ("%s requires a value for input '%s' because one of %s " - "is set. For a list of required inputs, see %s.help()" % - (self.__class__.__name__, name, - ', '.join(spec.requires), self.__class__.__name__)) + if len(values) > 1: + fmt = ("%s requires values for inputs %s because '%s' is set. " + "For a list of required inputs, see %s.help()") + else: + fmt = ("%s requires a value for input %s because '%s' is set. " + "For a list of required inputs, see %s.help()") + msg = fmt % (self.__class__.__name__, + ', '.join("'%s'" % req for req in spec.requires), + name, + self.__class__.__name__) raise ValueError(msg) def _check_xor(self, spec, name, value): From b706af826015d3a6039dcb1007c660300dcbcc8e Mon Sep 17 00:00:00 2001 From: "Christopher J. Markiewicz" Date: Thu, 14 Feb 2019 10:31:35 -0500 Subject: [PATCH 5/6] TEST: Thorough test of LaplacianThickness requirement cascade --- .../interfaces/ants/tests/test_segmentation.py | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/nipype/interfaces/ants/tests/test_segmentation.py b/nipype/interfaces/ants/tests/test_segmentation.py index d595317713..e9a1443934 100644 --- a/nipype/interfaces/ants/tests/test_segmentation.py +++ b/nipype/interfaces/ants/tests/test_segmentation.py @@ -42,8 +42,18 @@ def test_LaplacianThickness_defaults(change_dir, create_lt): def test_LaplacianThickness_wrongargs(change_dir, create_lt): lt = create_lt + lt.inputs.tolerance = 0.001 + with pytest.raises(ValueError, match=r".* requires a value for input 'sulcus_prior' .*"): + lt.cmdline + lt.inputs.sulcus_prior = 0.15 + with pytest.raises(ValueError, match=r".* requires a value for input 'dT' .*"): + lt.cmdline + lt.inputs.dT = 0.01 + with pytest.raises(ValueError, match=r".* requires a value for input 'prior_thickness' .*"): + lt.cmdline lt.inputs.prior_thickness = 5.9 - # 500 must not be placed as smooth_param - assert lt.cmdline != 'LaplacianThickness functional.nii diffusion_weighted.nii functional_thickness.nii 5.9' - # probably should have just raised an exception that "smooth_param" - # should also be defined \ No newline at end of file + with pytest.raises(ValueError, match=r".* requires a value for input 'smooth_param' .*"): + lt.cmdline + lt.inputs.smooth_param = 4.5 + assert lt.cmdline == 'LaplacianThickness functional.nii diffusion_weighted.nii ' \ + 'functional_thickness.nii 4.5 5.9 0.01 0.15 0.001' From 2ff466e68d1ae01a5f88dc8c9b75e8ca8ed95399 Mon Sep 17 00:00:00 2001 From: Yaroslav Halchenko Date: Fri, 15 Feb 2019 14:25:28 -0500 Subject: [PATCH 6/6] BF: regenerated test_auto_LaplacianThickness using wonderfully long running tools/checkspecs.py --- .../ants/tests/test_auto_LaplacianThickness.py | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/nipype/interfaces/ants/tests/test_auto_LaplacianThickness.py b/nipype/interfaces/ants/tests/test_auto_LaplacianThickness.py index 1bb82f0e33..608ba10889 100644 --- a/nipype/interfaces/ants/tests/test_auto_LaplacianThickness.py +++ b/nipype/interfaces/ants/tests/test_auto_LaplacianThickness.py @@ -7,8 +7,9 @@ def test_LaplacianThickness_inputs(): input_map = dict( args=dict(argstr='%s', ), dT=dict( - argstr='%f', + argstr='%s', position=6, + requires=['prior_thickness'], ), environ=dict( nohash=True, @@ -39,20 +40,23 @@ def test_LaplacianThickness_inputs(): position=3, ), prior_thickness=dict( - argstr='%f', + argstr='%s', position=5, + requires=['smooth_param'], ), smooth_param=dict( - argstr='%f', + argstr='%s', position=4, ), sulcus_prior=dict( - argstr='%f', + argstr='%s', position=7, + requires=['dT'], ), tolerance=dict( - argstr='%f', + argstr='%s', position=8, + requires=['sulcus_prior'], ), ) inputs = LaplacianThickness.input_spec()