diff --git a/plumpy/ports.py b/plumpy/ports.py index 78ee8883..61a0d194 100644 --- a/plumpy/ports.py +++ b/plumpy/ports.py @@ -6,18 +6,26 @@ import json import logging import six +import warnings from plumpy.utils import is_mutable_property, type_check if six.PY2: import collections + from inspect import getargspec as get_arg_spec else: import collections.abc as collections + from inspect import getfullargspec as get_arg_spec _LOGGER = logging.getLogger(__name__) UNSPECIFIED = () -__all__ = ['UNSPECIFIED', 'ValueSpec', 'PortValidationError', 'Port', 'InputPort', 'OutputPort'] +__all__ = ['UNSPECIFIED', 'PortValidationError', 'Port', 'InputPort', 'OutputPort'] + + +VALIDATOR_SIGNATURE_DEPRECATION_WARNING = """the validator `{}` has a signature that only takes a single argument. + This has been deprecated and the new signature is `validator(value, port)` where the `port` argument will be the + port instance to which the validator has been assigned.""" class PortValidationError(Exception): @@ -55,7 +63,8 @@ def port(self): return self._port -class ValueSpec(object): +@six.add_metaclass(abc.ABCMeta) +class Port(object): """ Specifications relating to a general input/output value including properties like whether it is required, valid types, the help string, etc. @@ -69,8 +78,7 @@ def __init__(self, name, valid_type=None, help=None, required=True, validator=No self._validator = validator def __str__(self): - """ - Get the string representing this value specification + """Get the string representing this port. :return: the string representation :rtype: str @@ -78,10 +86,9 @@ def __str__(self): return json.dumps(self.get_description()) def get_description(self): - """ - Return a description of the ValueSpec, which will be a dictionary of its attributes + """Return a description of the Port, which will be a dictionary of its attributes - :returns: a dictionary of the stringified ValueSpec attributes + :returns: a dictionary of the stringified Port attributes :rtype: dict """ description = { @@ -102,178 +109,78 @@ def name(self): @property def valid_type(self): - return self._valid_type - - @valid_type.setter - def valid_type(self, valid_type): - self._valid_type = valid_type - - @property - def help(self): - return self._help - - @help.setter - def help(self, help): # pylint: disable=redefined-builtin - self._help = help - - @property - def required(self): - return self._required - - @required.setter - def required(self, required): - self._required = required - - @property - def validator(self): - return self._validator - - @validator.setter - def validator(self, validator): - self._validator = validator - - def validate(self, value): - """ - Validate the value against this specification - - :param value: the value to validate - :type value: typing.Any - :return: if validation fails, returns a string containing the error message, otherwise None - :rtype: typing.Optional[str] - """ - if value is UNSPECIFIED: - if self._required: - return "required value was not provided for '{}'".format(self.name) - else: - if self._valid_type is not None and not isinstance(value, self._valid_type): - msg = "value '{}' is not of the right type. Got '{}', expected '{}'".format( - self.name, type(value), self._valid_type) - return msg - - if self._validator is not None: - result = self._validator(value) - if result is not None: - assert isinstance(result, str), "Validator returned non string type" - return result - - return - - -@six.add_metaclass(abc.ABCMeta) -class Port(object): - """ - Specifications relating to a general input/output value including - properties like whether it is required, valid types, the help string, etc. - """ - - def __init__(self, name, valid_type=None, help=None, required=True, validator=None): - self._value_spec = ValueSpec(name, valid_type, help, required, validator) - - def __str__(self): - """ - Get the string representing this value specification - - :return: the string representation - :rtype: str - """ - return str(self._value_spec) - - def get_description(self): - """ - Return a description of the ValueSpec, which will be a dictionary of its attributes - - :returns: a dictionary of the stringified ValueSpec attributes - :rtype: dict - """ - return self._value_spec.get_description() - - @property - def name(self): - return self._value_spec.name - - @property - def valid_type(self): - """ - Get the valid value type for this port if one is specified + """Get the valid value type for this port if one is specified :return: the value value type :rtype: type """ - return self._value_spec.valid_type + return self._valid_type @valid_type.setter def valid_type(self, valid_type): - """ - Set the valid value type for this port + """Set the valid value type for this port :param valid_type: the value valid type :type valid_type: type """ - self._value_spec.valid_type = valid_type + self._valid_type = valid_type @property def help(self): - """ - Get the help string for this port + """Get the help string for this port :return: the help string :rtype: str """ - return self._value_spec.help + return self._help @help.setter def help(self, help): - """ - Set the help string for this port + """Set the help string for this port :param help: the help string :type help: str """ - self._value_spec.help = help + self._help = help @property def required(self): - """ - Is this port required? + """Is this port required? :return: True if required, False otherwise :rtype: bool """ - return self._value_spec.required + return self._required @required.setter def required(self, required): - """ - Set whether this port is required or not + """Set whether this port is required or not :return: True if required, False otherwise :type required: bool """ - self._value_spec.required = required + self._required = required @property def validator(self): - """ - Get the validator for this port + """Get the validator for this port :return: the validator :rtype: typing.Callable[[typing.Any], typing.Tuple[bool, typing.Optional[str]]] """ - return self._value_spec.validator + return self._validator @validator.setter def validator(self, validator): - """ - Set the validator for this port + """Set the validator for this port :param validator: a validator function :type validator: typing.Callable[[typing.Any], typing.Tuple[bool, typing.Optional[str]]] """ - self._value_spec.validator = validator + self._validator = validator def validate(self, value, breadcrumbs=()): - """ - Validate a value to see if it is valid for this port + """Validate a value to see if it is valid for this port :param value: the value to check :type value: typing.Any @@ -282,7 +189,25 @@ def validate(self, value, breadcrumbs=()): :return: None or tuple containing 0: error string 1: tuple of breadcrumb strings to where the validation failed :rtype: typing.Optional[ValidationError] """ - validation_error = self._value_spec.validate(value) + validation_error = None + + if value is UNSPECIFIED and self._required: + validation_error = "required value was not provided for '{}'".format(self.name) + elif value is not UNSPECIFIED and self._valid_type is not None and not isinstance(value, self._valid_type): + validation_error = "value '{}' is not of the right type. Got '{}', expected '{}'".format( + self.name, type(value), self._valid_type) + + if not validation_error and self._validator is not None: + spec = get_arg_spec(self.validator) + if len(spec[0]) == 1: + warnings.warn(VALIDATOR_SIGNATURE_DEPRECATION_WARNING.format(self.validator.__name__)) + result = self.validator(value) + else: + result = self.validator(value, self) + if result is not None: + assert isinstance(result, str), "Validator returned non string type" + validation_error = result + if validation_error: breadcrumbs += (self.name,) return PortValidationError(validation_error, breadcrumbs_to_port(breadcrumbs)) @@ -694,7 +619,12 @@ def validate(self, port_values=None, breadcrumbs=()): # Validate the validator after the ports themselves, as it most likely will rely on the port values if self.validator is not None: - message = self.validator(port_values_clone) + spec = get_arg_spec(self.validator) + if len(spec[0]) == 1: + warnings.warn(VALIDATOR_SIGNATURE_DEPRECATION_WARNING.format(self.validator.__name__)) + message = self.validator(port_values_clone) + else: + message = self.validator(port_values_clone, self) if message is not None: assert isinstance(message, str), \ "Validator returned something other than None or str: '{}'".format(type(message)) diff --git a/test/test_expose.py b/test/test_expose.py index 19118bc6..c97b96ee 100644 --- a/test/test_expose.py +++ b/test/test_expose.py @@ -12,7 +12,7 @@ class TestExposeProcess(utils.TestCaseWithLoop): def setUp(self): super(TestExposeProcess, self).setUp() - def validator_function(input): + def validator_function(input, port): pass class BaseNamespaceProcess(NewLoopProcess): @@ -72,12 +72,7 @@ def check_namespace_properties(self, process_left, namespace_left, process_right port_namespace_left.__dict__.pop('_ports', None) port_namespace_right.__dict__.pop('_ports', None) - # The `_value_spec` is a nested dictionary so should be compared explicitly separately - value_spec_left = port_namespace_left._value_spec - value_spec_right = port_namespace_right._value_spec - self.assertEqual(port_namespace_left.__dict__, port_namespace_right.__dict__) - self.assertEqual(value_spec_left.__dict__, value_spec_right.__dict__) def test_expose_nested_namespace(self): """Test that expose_inputs can create nested namespaces while maintaining own ports.""" @@ -188,7 +183,7 @@ def test_expose_ports_top_level(self): properties with that of the exposed process """ - def validator_function(input): + def validator_function(input, port): pass # Define child process with all mutable properties of the inputs PortNamespace to a non-default value @@ -235,7 +230,7 @@ def test_expose_ports_top_level_override(self): namespace_options will be the end-all-be-all """ - def validator_function(input): + def validator_function(input, port): pass # Define child process with all mutable properties of the inputs PortNamespace to a non-default value @@ -288,7 +283,7 @@ def test_expose_ports_namespace(self): namespace with the properties of the exposed port namespace """ - def validator_function(input): + def validator_function(input, port): pass # Define child process with all mutable properties of the inputs PortNamespace to a non-default value diff --git a/test/test_port.py b/test/test_port.py index aa3e830b..ba123c99 100644 --- a/test/test_port.py +++ b/test/test_port.py @@ -3,7 +3,35 @@ import types from .utils import TestCase -from plumpy.ports import InputPort, OutputPort, PortNamespace +from plumpy.ports import Port, InputPort, OutputPort, PortNamespace, UNSPECIFIED + + +class TestPort(TestCase): + + def test_required(self): + spec = Port("required_value", required=True) + + self.assertIsNotNone(spec.validate(UNSPECIFIED)) + self.assertIsNone(spec.validate(5)) + + def test_validate(self): + spec = Port("required_value", valid_type=int) + + self.assertIsNone(spec.validate(5)) + self.assertIsNotNone(spec.validate('a')) + + def test_validator(self): + + def validate(value, port): + assert isinstance(port, Port) + if not isinstance(value, int): + return "Not int" + return None + + spec = Port("valid_with_validator", validator=validate) + + self.assertIsNone(spec.validate(5)) + self.assertIsNotNone(spec.validate('s')) class TestInputPort(TestCase): @@ -19,7 +47,8 @@ def test_default(self): def test_validator(self): """Test the validator functionality.""" - def integer_validator(value): + def integer_validator(value, port): + assert isinstance(port, Port) if value < 0: return 'Only positive integers allowed' @@ -57,8 +86,8 @@ def test_default(self): help_string = 'Help string' required = False - def validator(value): - pass + def validator(value, port): + assert isinstance(port, Port) port = OutputPort(name, valid_type=valid_type, help=help_string, required=required, validator=validator) self.assertEqual(port.name, name) @@ -93,7 +122,8 @@ def test_port_namespace(self): def test_port_namespace_validation(self): """Test validate method of a `PortNamespace`.""" - def validator(port_values): + def validator(port_values, port): + assert isinstance(port, PortNamespace) if port_values['explicit'] < 0 or port_values['dynamic'] < 0: return 'Only positive integers allowed' diff --git a/test/test_process_spec.py b/test/test_process_spec.py index ae14253f..28b064e5 100644 --- a/test/test_process_spec.py +++ b/test/test_process_spec.py @@ -75,7 +75,7 @@ def test_input_namespaced(self): def test_validator(self): """Test the port validator with default.""" - def dict_validator(dictionary): + def dict_validator(dictionary, port): if 'key' not in dictionary or dictionary['key'] is not 'value': return 'Invalid dictionary' @@ -94,7 +94,7 @@ def dict_validator(dictionary): def test_validate(self): """Test the global spec validator functionality.""" - def is_valid(inputs): + def is_valid(inputs, port): if not ('a' in inputs) ^ ('b' in inputs): return 'Must have a OR b in inputs' return diff --git a/test/test_value_spec.py b/test/test_value_spec.py deleted file mode 100644 index ee3037d0..00000000 --- a/test/test_value_spec.py +++ /dev/null @@ -1,31 +0,0 @@ -from __future__ import absolute_import -import plumpy - -from . import utils - - -class TestValueSpec(utils.TestCase): - - def test_required(self): - spec = plumpy.ValueSpec("required_value", required=True) - - self.assertIsNotNone(spec.validate(plumpy.UNSPECIFIED)) - self.assertIsNone(spec.validate(5)) - - def test_validate(self): - spec = plumpy.ValueSpec("required_value", valid_type=int) - - self.assertIsNone(spec.validate(5)) - self.assertIsNotNone(spec.validate('a')) - - def test_validator(self): - - def validate(value): - if not isinstance(value, int): - return "Not int" - return None - - spec = plumpy.ValueSpec("valid_with_validator", validator=validate) - - self.assertIsNone(spec.validate(5)) - self.assertIsNotNone(spec.validate('s'))