Merge pull request #263 from seequent/beta

v0.5.5 Deployment - Better validation error messages

.bumpversion.cfg

@@ -1,4 +1,4 @@
 [bumpversion]
-current_version = 0.5.4
+current_version = 0.5.5
 files = properties/__init__.py setup.py docs/conf.py
 

docs/conf.py

@@ -64,9 +64,9 @@
 # built documents.
 #
 # The short X.Y version.
-version = u'0.5.4'
+version = u'0.5.5'
 # The full version, including alpha/beta/rc tags.
-release = u'0.5.4'
+release = u'0.5.5'
 
 # The language for content autogenerated by Sphinx. Refer to documentation
 # for a list of supported languages.

properties/__init__.py

@@ -89,7 +89,7 @@ class Profile(properties.HasProperties):
     ValidationError,
 )
 
-__version__ = '0.5.4'
+__version__ = '0.5.5'
 __author__ = 'Seequent'
 __license__ = 'MIT'
 __copyright__ = 'Copyright 2018 Seequent'

properties/base/containers.py

@@ -289,10 +289,16 @@ def assert_valid(self, instance, value=None):
             value = instance._get(self.name)
             if value is None:
                 return True
-        if self.min_length is not None and len(value) < self.min_length:
-            self.error(instance, value)
-        if self.max_length is not None and len(value) > self.max_length:
-            self.error(instance, value)
+        if (
+                (self.min_length is not None and len(value) < self.min_length)
+                or
+                (self.max_length is not None and len(value) > self.max_length)
+        ):
+            self.error(
+                instance=instance,
+                value=value,
+                extra='(Length is {})'.format(len(value)),
+            )
         for val in value:
             if not self.prop.assert_valid(instance, val):
                 return False
@@ -567,8 +573,12 @@ def validate(self, instance, value):
         if self.coerce:
             try:
                 value = self._class_container(value)
-            except TypeError:
-                self.error(instance, value)
+            except (TypeError, ValueError):
+                self.error(
+                    instance=instance,
+                    value=value,
+                    extra='Cannot coerce to the correct type',
+                )
         out = value.__class__()
         for key, val in iteritems(value):
             if self.key_prop:

properties/base/instance.py

@@ -9,7 +9,7 @@
 
 from six import PY2
 
-from .base import HasProperties, equal
+from .base import GENERIC_ERRORS, HasProperties, equal
 from .. import basic
 from .. import utils
 
@@ -101,8 +101,14 @@ def validate(self, instance, value):
             if isinstance(value, dict):
                 return self.instance_class(**value)
             return self.instance_class(value)
-        except (ValueError, KeyError, TypeError):
-            self.error(instance, value)
+        except GENERIC_ERRORS as err:
+            if hasattr(err, 'error_tuples'):
+                extra = '({})'.format(' & '.join(
+                    err_tup.message for err_tup in err.error_tuples
+                ))
+            else:
+                extra = ''
+            self.error(instance, value, extra=extra)
 
     def assert_valid(self, instance, value=None):
         """Checks if valid, including HasProperty instances pass validation"""

properties/base/union.py

@@ -8,7 +8,8 @@
 
 from six import PY2
 
-from ..base import GENERIC_ERRORS, HasProperties, Instance
+from .base import GENERIC_ERRORS, HasProperties
+from .instance import Instance
 from .. import basic
 from .. import utils
 
@@ -160,14 +161,32 @@ def _unused_default_warning(self):
                 warn('Union prop default ignored: {}'.format(prop.default),
                      RuntimeWarning)
 
-    def validate(self, instance, value):
-        """Check if value is a valid type of one of the Union props"""
+    def _try_prop_method(self, instance, value, method_name):
+        """Helper method to perform a method on each of the union props
+
+        This method gathers all errors and returns them at the end
+        if the method on each of the props fails.
+        """
+        error_messages = []
         for prop in self.props:
             try:
-                return prop.validate(instance, value)
-            except GENERIC_ERRORS:
-                continue
-        self.error(instance, value)
+                return getattr(prop, method_name)(instance, value)
+            except GENERIC_ERRORS as err:
+                if hasattr(err, 'error_tuples'):
+                    error_messages += [
+                        err_tup.message for err_tup in err.error_tuples
+                    ]
+        if error_messages:
+            extra = 'Possible explanation:'
+            for message in error_messages:
+                extra += '\n    - {}'.format(message)
+        else:
+            extra = ''
+        self.error(instance, value, extra=extra)
+
+    def validate(self, instance, value):
+        """Check if value is a valid type of one of the Union props"""
+        return self._try_prop_method(instance, value, 'validate')
 
     def assert_valid(self, instance, value=None):
         """Check if the Union has a valid value"""
@@ -178,19 +197,7 @@ def assert_valid(self, instance, value=None):
             value = instance._get(self.name)
             if value is None:
                 return True
-        for prop in self.props:
-            try:
-                return prop.assert_valid(instance, value)
-            except GENERIC_ERRORS:
-                continue
-        message = (
-            'The "{name}" property of a {cls} instance has not been set '
-            'correctly'.format(
-                name=self.name,
-                cls=instance.__class__.__name__
-            )
-        )
-        raise utils.ValidationError(message, 'invalid', self.name, instance)
+        return self._try_prop_method(instance, value, 'assert_valid')
 
     def serialize(self, value, **kwargs):
         """Return a serialized value

properties/basic.py

@@ -6,7 +6,6 @@
 
 import collections
 import datetime
-from functools import wraps
 import math
 import random
 import re
@@ -45,7 +44,6 @@ def accept_kwargs(func):
     functions always receive kwargs from serialize, but by using this,
     the original functions may simply take a single value.
     """
-    @wraps(func)
     def wrapped(val, **kwargs):
         """Perform a function on a value, ignoring kwargs if necessary"""
         try:
@@ -339,14 +337,19 @@ def error(self, instance, value, error_class=None, extra=''):
             prefix = prefix + ' of a {cls} instance'.format(
                 cls=instance.__class__.__name__,
             )
+        print_value = repr(value)
+        if len(print_value) > 107:
+            print_value = '{}  ...  {}'.format(
+                print_value[:50], print_value[-50:]
+            )
         message = (
-            '{prefix} must be {info}. A value of {val!r} {vtype!r} was '
-            'specified. {extra}'.format(
+            '{prefix} must be {info}. An invalid value of {val} {vtype} was '
+            'specified.{extra}'.format(
                 prefix=prefix,
                 info=self.info or 'corrected',
-                val=value,
+                val=print_value,
                 vtype=type(value),
-                extra=extra,
+                extra=' {}'.format(extra) if extra else '',
             )
         )
         if issubclass(error_class, ValidationError):
@@ -732,7 +735,7 @@ def validate(self, instance, value):
             try:
                 value = bool(value)
             except ValueError:
-                self.error(instance, value)
+                self.error(instance, value, extra='Cannot cast to boolean.')
         if not isinstance(value, BOOLEAN_TYPES):
             self.error(instance, value)
         return value
@@ -765,7 +768,7 @@ def _in_bounds(prop, instance, value):
             (prop.min is not None and value < prop.min) or
             (prop.max is not None and value > prop.max)
     ):
-        prop.error(instance, value)
+        prop.error(instance, value, extra='Not within allowed range.')
 
 
 class Integer(Boolean):
@@ -811,9 +814,13 @@ def validate(self, instance, value):
         try:
             intval = int(value)
             if not self.cast and abs(value - intval) > TOL:
-                self.error(instance, value)
+                self.error(
+                    instance=instance,
+                    value=value,
+                    extra='Not within tolerance range of {}.'.format(TOL),
+                )
         except (TypeError, ValueError):
-            self.error(instance, value)
+            self.error(instance, value, extra='Cannot cast to integer.')
         _in_bounds(self, instance, intval)
         return intval
 
@@ -861,9 +868,13 @@ def validate(self, instance, value):
         try:
             floatval = float(value)
             if not self.cast and abs(value - floatval) > TOL:
-                self.error(instance, value)
+                self.error(
+                    instance=instance,
+                    value=value,
+                    extra='Not within tolerance range of {}.'.format(TOL),
+                )
         except (TypeError, ValueError):
-            self.error(instance, value)
+            self.error(instance, value, extra='Cannot cast to float.')
         _in_bounds(self, instance, floatval)
         return floatval
 
@@ -907,7 +918,11 @@ def validate(self, instance, value):
                     abs(value.real - compval.real) > TOL or
                     abs(value.imag - compval.imag) > TOL
             ):
-                self.error(instance, value)
+                self.error(
+                    instance=instance,
+                    value=value,
+                    extra='Not within tolerance range of {}.'.format(TOL),
+                )
         except (TypeError, ValueError, AttributeError):
             self.error(instance, value)
         return compval
@@ -1012,7 +1027,7 @@ def validate(self, instance, value):
         if not isinstance(value, string_types):
             self.error(instance, value)
         if self.regex is not None and self.regex.search(value) is None:        #pylint: disable=no-member
-            self.error(instance, value)
+            self.error(instance, value, extra='Regex does not match.')
         value = value.strip(self.strip)
         if self.change_case == 'upper':
             value = value.upper()
@@ -1153,7 +1168,7 @@ def validate(self, instance, value):                                       #pyli
             test_val = val if self.case_sensitive else [_.upper() for _ in val]
             if test_value == test_key or test_value in test_val:
                 return key
-        self.error(instance, value)
+        self.error(instance, value, extra='Not an available choice.')
 
 
 class Color(Property):
@@ -1226,11 +1241,19 @@ def validate(self, instance, value):
         if isinstance(value, datetime.datetime):
             return value
         if not isinstance(value, string_types):
-            self.error(instance, value)
+            self.error(
+                instance=instance,
+                value=value,
+                extra='Cannot convert non-strings to datetime.',
+            )
         try:
             return self.from_json(value)
         except ValueError:
-            self.error(instance, value)
+            self.error(
+                instance=instance,
+                value=value,
+                extra='Invalid format for converting to datetime.',
+            )
 
     @staticmethod
     def to_json(value, **kwargs):

properties/extras/web.py

@@ -48,7 +48,7 @@ def validate(self, instance, value):
         value = super(URL, self).validate(instance, value)
         parsed_url = urlparse(value)
         if not parsed_url.scheme or not parsed_url.netloc:
-            self.error(instance, value)
+            self.error(instance, value, extra='URL needs scheme and netloc.')
         parse_result = ParseResult(
             scheme=parsed_url.scheme,
             netloc=parsed_url.netloc,

properties/math.py

@@ -133,7 +133,7 @@ def validate(self, instance, value):
                 'subclasses of numpy.ndarray'
             )
         if value.dtype.kind not in (TYPE_MAPPINGS[typ] for typ in self.dtype):
-            self.error(instance, value)
+            self.error(instance, value, extra='Invalid dtype.')
         if self.shape is None:
             return value
         for shape in self.shape:
@@ -144,7 +144,7 @@ def validate(self, instance, value):
                     break
             else:
                 return value
-        self.error(instance, value)
+        self.error(instance, value, extra='Invalid shape.')
 
     def equal(self, value_a, value_b):
         try:
@@ -421,7 +421,11 @@ def validate(self, instance, value):
         for i, val in enumerate(value):
             if isinstance(val, string_types):
                 if val.upper() not in VECTOR_DIRECTIONS:
-                    self.error(instance, val)
+                    self.error(
+                        instance=instance,
+                        value=val,
+                        extra='This is an invalid Vector3 representation.',
+                    )
                 value[i] = VECTOR_DIRECTIONS[val.upper()]
 
         return super(Vector3Array, self).validate(instance, value)
@@ -482,11 +486,16 @@ def validate(self, instance, value):
             self.error(instance, value)
         if isinstance(value, (tuple, list)):
             for i, val in enumerate(value):
-                if (
-                        isinstance(val, string_types) and
-                        val.upper() in VECTOR_DIRECTIONS and
-                        val.upper() not in ('Z', '-Z', 'UP', 'DOWN')
-                ):
+                if isinstance(val, string_types):
+                    if (
+                            val.upper() not in VECTOR_DIRECTIONS or
+                            val.upper() in ('Z', '-Z', 'UP', 'DOWN')
+                    ):
+                        self.error(
+                            instance=instance,
+                            value=val,
+                            extra='This is an invalid Vector2 representation.',
+                        )
                     value[i] = VECTOR_DIRECTIONS[val.upper()][:2]
 
         return super(Vector2Array, self).validate(instance, value)

setup.py

@@ -29,7 +29,7 @@
 EXTRAS.update({'full': sum(EXTRAS.values(), [])})
 setup(
     name='properties',
-    version='0.5.4',
+    version='0.5.5',
     packages=find_packages(exclude=('tests',)),
     install_requires=['six>=1.7.3'],
     extras_require=EXTRAS,

tests/test_container.py

@@ -941,6 +941,9 @@ class HasCoercedDict(properties.HasProperties):
         hcd.my_coerced_dict = key_val_list
         assert hcd.my_coerced_dict == {'a': 1, 'b': 2, 'c': 3}
 
+        with self.assertRaises(properties.ValidationError):
+            hcd.my_coerced_dict = 'a'
+
     def test_nested_observed(self):
         self._test_nested_observed(True)
         self._test_nested_observed(False)