Skip to content
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

handle unavailable traits due to version differences #3273

Merged
merged 6 commits into from Nov 23, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
29 changes: 22 additions & 7 deletions nipype/interfaces/base/core.py
Expand Up @@ -31,7 +31,7 @@

from ...external.due import due

from .traits_extension import traits, isdefined
from .traits_extension import traits, isdefined, Undefined
from .specs import (
BaseInterfaceInputSpec,
CommandLineInputSpec,
Expand Down Expand Up @@ -180,7 +180,16 @@ def __init__(
if not self.input_spec:
raise Exception("No input_spec in class: %s" % self.__class__.__name__)

self.inputs = self.input_spec(**inputs)
# Create input spec, disable any defaults that are unavailable due to
# version, and then apply the inputs that were passed.
self.inputs = self.input_spec()
unavailable_traits = self._check_version_requirements(
self.inputs, permissive=True
)
if unavailable_traits:
self.inputs.trait_set(**{k: Undefined for k in unavailable_traits})
self.inputs.trait_set(**inputs)

self.ignore_exception = ignore_exception

if resource_monitor is not None:
Expand Down Expand Up @@ -264,8 +273,12 @@ def _check_mandatory_inputs(self):
):
self._check_requires(spec, name, getattr(self.inputs, name))

def _check_version_requirements(self, trait_object, raise_exception=True):
def _check_version_requirements(self, trait_object, permissive=False):
""" Raises an exception on version mismatch

Set the ``permissive`` attribute to True to suppress warnings and exceptions.
This is currently only used in __init__ to silently identify unavailable
traits.
"""
unavailable_traits = []
# check minimum version
Expand All @@ -283,15 +296,16 @@ def _check_version_requirements(self, trait_object, raise_exception=True):
f"Nipype cannot validate the package version {version!r} for "
f"{self.__class__.__name__}. Trait {name} requires version >={min_ver}."
)
iflogger.warning(f"{msg}. Please verify validity.")
if not permissive:
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)
if not isdefined(getattr(trait_object, name)):
continue
if raise_exception:
if not permissive:
raise Exception(
"Trait %s (%s) (version %s < required %s)"
% (name, self.__class__.__name__, version, min_ver)
Expand All @@ -311,15 +325,16 @@ def _check_version_requirements(self, trait_object, raise_exception=True):
f"Nipype cannot validate the package version {version!r} for "
f"{self.__class__.__name__}. Trait {name} requires version <={max_ver}."
)
iflogger.warning(f"{msg}. Please verify validity.")
if not permissive:
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)
if not isdefined(getattr(trait_object, name)):
continue
if raise_exception:
if not permissive:
raise Exception(
"Trait %s (%s) (version %s > required %s)"
% (name, self.__class__.__name__, version, max_ver)
Expand Down
58 changes: 50 additions & 8 deletions nipype/interfaces/base/tests/test_core.py
Expand Up @@ -253,7 +253,7 @@ class input_spec(nib.TraitedSpec):
assert len(caplog.records) == 2


def test_input_version_missing_error():
def test_input_version_missing_error(caplog):
from nipype import config

class DerivedInterface(nib.BaseInterface):
Expand All @@ -263,13 +263,55 @@ class input_spec(nib.TraitedSpec):

_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)
obj1 = DerivedInterface(foo=1)
obj2 = DerivedInterface(bar=1)
with caplog.at_level(logging.WARNING, logger="nipype.interface"):
with mock.patch.object(config, "getboolean", return_value=True):
with pytest.raises(ValueError):
obj1._check_version_requirements(obj1.inputs)
with pytest.raises(ValueError):
obj2._check_version_requirements(obj2.inputs)
assert len(caplog.records) == 2


def test_unavailable_input():
class WithInput(nib.BaseInterface):
class input_spec(nib.TraitedSpec):
foo = nib.traits.Int(3, usedefault=True, max_ver="0.5")

_version = "0.4"

def _run_interface(self, runtime):
return runtime

class WithoutInput(WithInput):
_version = "0.6"

has = WithInput()
hasnt = WithoutInput()
trying_anyway = WithoutInput(foo=3)
assert has.inputs.foo == 3
assert not nib.isdefined(hasnt.inputs.foo)
assert trying_anyway.inputs.foo == 3

has.run()
hasnt.run()
with pytest.raises(Exception):
trying_anyway.run()

# Still settable
has.inputs.foo = 4
hasnt.inputs.foo = 4
trying_anyway.inputs.foo = 4
assert has.inputs.foo == 4
assert hasnt.inputs.foo == 4
assert trying_anyway.inputs.foo == 4

has.run()
with pytest.raises(Exception):
hasnt.run()
with pytest.raises(Exception):
trying_anyway.run()


def test_output_version():
Expand Down
2 changes: 1 addition & 1 deletion nipype/interfaces/tests/test_io.py
Expand Up @@ -465,7 +465,7 @@ def test_datasink_substitutions(tmpdir):
files.append(f)
open(f, "w")
ds = nio.DataSink(
parametrization=False,
parameterization=False,
base_directory=str(outdir),
substitutions=[("ababab", "ABABAB")],
# end archoring ($) is used to assure operation on the filename
Expand Down