From 44497f278a2be55c301a46e6eaeeecb5762a52db Mon Sep 17 00:00:00 2001 From: Horea Christian Date: Fri, 7 Aug 2020 01:12:38 -0400 Subject: [PATCH 1/4] ENH: no more auto-failing on misparsed versions --- nipype/interfaces/base/core.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/nipype/interfaces/base/core.py b/nipype/interfaces/base/core.py index 82da393a84..1e626fe1b5 100644 --- a/nipype/interfaces/base/core.py +++ b/nipype/interfaces/base/core.py @@ -276,6 +276,15 @@ def _check_version_requirements(self, trait_object, raise_exception=True): version = LooseVersion(str(self.version)) for name in names: min_ver = LooseVersion(str(trait_object.traits()[name].min_ver)) + try: + min_ver > version + except TypeError: + iflogger.warning( + 'Nipype is having issues parsing the package version ' + f'for Trait {name} ({self.__class__.__name__})' + f'You may want to check whether {version} is larger than {min_ver}' + ) + continue if min_ver > version: unavailable_traits.append(name) if not isdefined(getattr(trait_object, name)): @@ -293,6 +302,15 @@ def _check_version_requirements(self, trait_object, raise_exception=True): version = LooseVersion(str(self.version)) for name in names: max_ver = LooseVersion(str(trait_object.traits()[name].max_ver)) + try: + max_ver > version + except TypeError: + iflogger.warning( + 'Nipype is having issues parsing the package version ' + f'for Trait {name} ({self.__class__.__name__})' + f'You may want to check whether {version} is smaller than {max_ver}' + ) + continue if max_ver < version: unavailable_traits.append(name) if not isdefined(getattr(trait_object, name)): From 690c620705288a3be11e28ac1703c555b12fd9a7 Mon Sep 17 00:00:00 2001 From: Chris Markiewicz Date: Fri, 14 Aug 2020 17:07:18 -0400 Subject: [PATCH 2/4] TEST: Validate bad version parse warning when hit with min/max version --- nipype/interfaces/base/tests/test_core.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/nipype/interfaces/base/tests/test_core.py b/nipype/interfaces/base/tests/test_core.py index e97a5bab79..e3dd5cf1de 100644 --- a/nipype/interfaces/base/tests/test_core.py +++ b/nipype/interfaces/base/tests/test_core.py @@ -3,6 +3,7 @@ # vi: set ft=python sts=4 ts=4 sw=4 et: import os import simplejson as json +import logging import pytest from unittest import mock @@ -236,6 +237,21 @@ class DerivedInterface1(nib.BaseInterface): obj._check_version_requirements(obj.inputs) +def test_input_version_missing(caplog): + class DerivedInterface(nib.BaseInterface): + class input_spec(nib.TraitedSpec): + foo = nib.traits.Int(min_ver="0.9") + bar = nib.traits.Int(max_ver="0.9") + _version = "misparsed-garbage" + + obj = DerivedInterface() + obj.inputs.foo = 1 + obj.inputs.bar = 1 + with caplog.at_level(logging.WARNING, logger="nipype.interface"): + obj._check_version_requirements(obj.inputs) + assert len(caplog.records) == 2 + + def test_output_version(): class InputSpec(nib.TraitedSpec): foo = nib.traits.Int(desc="a random int") From c56a3b1d788b86ad402a45699d8588c09f87f3e8 Mon Sep 17 00:00:00 2001 From: Chris Markiewicz Date: Fri, 14 Aug 2020 17:13:53 -0400 Subject: [PATCH 3/4] STY: Reuse comparison, rewrite warning --- nipype/interfaces/base/core.py | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/nipype/interfaces/base/core.py b/nipype/interfaces/base/core.py index 1e626fe1b5..d03ea941b3 100644 --- a/nipype/interfaces/base/core.py +++ b/nipype/interfaces/base/core.py @@ -277,15 +277,15 @@ def _check_version_requirements(self, trait_object, raise_exception=True): for name in names: min_ver = LooseVersion(str(trait_object.traits()[name].min_ver)) try: - min_ver > version + too_old = min_ver > version except TypeError: iflogger.warning( - 'Nipype is having issues parsing the package version ' - f'for Trait {name} ({self.__class__.__name__})' - f'You may want to check whether {version} is larger than {min_ver}' - ) + f"Nipype cannot validate the package version {version!r} for " + f"{self.__class__.__name__}. Trait {name} requires version " + f">={min_ver}. Please verify validity." + ) continue - if min_ver > version: + if too_old: unavailable_traits.append(name) if not isdefined(getattr(trait_object, name)): continue @@ -303,15 +303,15 @@ def _check_version_requirements(self, trait_object, raise_exception=True): for name in names: max_ver = LooseVersion(str(trait_object.traits()[name].max_ver)) try: - max_ver > version + too_new = max_ver < version except TypeError: iflogger.warning( - 'Nipype is having issues parsing the package version ' - f'for Trait {name} ({self.__class__.__name__})' - f'You may want to check whether {version} is smaller than {max_ver}' - ) + f"Nipype cannot validate the package version {version!r} for " + f"{self.__class__.__name__}. Trait {name} requires version " + f"<={max_ver}. Please verify validity." + ) continue - if max_ver < version: + if too_new: unavailable_traits.append(name) if not isdefined(getattr(trait_object, name)): continue From bf24dbaf6a4bf8227de049879b69e053e854ac6c Mon Sep 17 00:00:00 2001 From: "Christopher J. Markiewicz" Date: Sun, 16 Aug 2020 12:58:00 -0400 Subject: [PATCH 4/4] RF: Raise ValueError if stop_on_unknown_version is set --- nipype/interfaces/base/core.py | 20 ++++++++++++-------- nipype/interfaces/base/tests/test_core.py | 22 +++++++++++++++++++++- 2 files changed, 33 insertions(+), 9 deletions(-) diff --git a/nipype/interfaces/base/core.py b/nipype/interfaces/base/core.py index d03ea941b3..54c4302c7f 100644 --- a/nipype/interfaces/base/core.py +++ b/nipype/interfaces/base/core.py @@ -278,12 +278,14 @@ def _check_version_requirements(self, trait_object, raise_exception=True): min_ver = LooseVersion(str(trait_object.traits()[name].min_ver)) try: too_old = min_ver > version - except TypeError: - iflogger.warning( + except TypeError as err: + msg = ( f"Nipype cannot validate the package version {version!r} for " - f"{self.__class__.__name__}. Trait {name} requires version " - f">={min_ver}. Please verify validity." + f"{self.__class__.__name__}. Trait {name} requires version >={min_ver}." ) + iflogger.warning(f"{msg}. Please verify validity.") + if config.getboolean("execution", "stop_on_unknown_version"): + raise ValueError(msg) from err continue if too_old: unavailable_traits.append(name) @@ -304,12 +306,14 @@ def _check_version_requirements(self, trait_object, raise_exception=True): max_ver = LooseVersion(str(trait_object.traits()[name].max_ver)) try: too_new = max_ver < version - except TypeError: - iflogger.warning( + except TypeError as err: + msg = ( f"Nipype cannot validate the package version {version!r} for " - f"{self.__class__.__name__}. Trait {name} requires version " - f"<={max_ver}. Please verify validity." + f"{self.__class__.__name__}. Trait {name} requires version <={max_ver}." ) + iflogger.warning(f"{msg}. Please verify validity.") + if config.getboolean("execution", "stop_on_unknown_version"): + raise ValueError(msg) from err continue if too_new: unavailable_traits.append(name) diff --git a/nipype/interfaces/base/tests/test_core.py b/nipype/interfaces/base/tests/test_core.py index e3dd5cf1de..165b3532ab 100644 --- a/nipype/interfaces/base/tests/test_core.py +++ b/nipype/interfaces/base/tests/test_core.py @@ -242,6 +242,7 @@ class DerivedInterface(nib.BaseInterface): class input_spec(nib.TraitedSpec): foo = nib.traits.Int(min_ver="0.9") bar = nib.traits.Int(max_ver="0.9") + _version = "misparsed-garbage" obj = DerivedInterface() @@ -252,6 +253,25 @@ class input_spec(nib.TraitedSpec): assert len(caplog.records) == 2 +def test_input_version_missing_error(): + from nipype import config + + class DerivedInterface(nib.BaseInterface): + class input_spec(nib.TraitedSpec): + foo = nib.traits.Int(min_ver="0.9") + bar = nib.traits.Int(max_ver="0.9") + + _version = "misparsed-garbage" + + with mock.patch.object(config, "getboolean", return_value=True): + obj = DerivedInterface(foo=1) + with pytest.raises(ValueError): + obj._check_version_requirements(obj.inputs) + obj = DerivedInterface(bar=1) + with pytest.raises(ValueError): + obj._check_version_requirements(obj.inputs) + + def test_output_version(): class InputSpec(nib.TraitedSpec): foo = nib.traits.Int(desc="a random int") @@ -473,7 +493,7 @@ def test_global_CommandLine_output(tmpdir): ci = BET() assert ci.terminal_output == "stream" # default case - with mock.patch.object(nib.CommandLine, '_terminal_output'): + with mock.patch.object(nib.CommandLine, "_terminal_output"): nib.CommandLine.set_default_terminal_output("allatonce") ci = nib.CommandLine(command="ls -l") assert ci.terminal_output == "allatonce"