From 456d6510ebb7c0bf07adee501f72d5a6ee145651 Mon Sep 17 00:00:00 2001 From: Sebastiaan Huber Date: Tue, 24 Dec 2019 11:22:36 +0100 Subject: [PATCH] Add context argument to `Port` validator method For normal `Port` instances, the `validator` signature changes from `validator(value)` to `validator(value, port)` where `port` is the instance of the `Port` to which the called validator is assigned. For `PortNamespace` validators the `port` argument is the instance of the port namespace itself. The `port` is especially useful in validators of namespaces because it can be used to see what ports are actually present. The expose functionality, which allows to exclude or include only certain ports can fundamentally change the ports that are present in a namespace, however, the validator remains untouced. To make the validator robust to these changes it should check if the port of interest is still present in the namespace before checking the corresponding value passed in the `values` argument. --- plumpy/ports.py | 22 ++++++++++++++++++++-- test/test_expose.py | 8 ++++---- test/test_port.py | 13 ++++++++----- test/test_process_spec.py | 4 ++-- 4 files changed, 34 insertions(+), 13 deletions(-) diff --git a/plumpy/ports.py b/plumpy/ports.py index be88198e..61a0d194 100644 --- a/plumpy/ports.py +++ b/plumpy/ports.py @@ -6,13 +6,16 @@ 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 = () @@ -20,6 +23,11 @@ __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): """Error when validation fails on a port""" @@ -190,7 +198,12 @@ def validate(self, value, breadcrumbs=()): self.name, type(value), self._valid_type) if not validation_error and self._validator is not None: - result = self.validator(value) + 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 @@ -606,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 f5a0f68b..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): @@ -183,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 @@ -230,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 @@ -283,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 f0926de9..ba123c99 100644 --- a/test/test_port.py +++ b/test/test_port.py @@ -22,7 +22,8 @@ def test_validate(self): def test_validator(self): - def validate(value): + def validate(value, port): + assert isinstance(port, Port) if not isinstance(value, int): return "Not int" return None @@ -46,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' @@ -84,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) @@ -120,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