Repository: ipython/traitlets
Pull request #262 (2016-08-02 07:10:51)

Bounded numeric traits
Prior to this PR, the `Integer` and `Long` traitlets with bounds did not actually validate that a value fell within the bounds in Python 2 (issue #259). This does not apply to Python 3, where Long and Integer are just aliases of `Int`.

In its current state, this PR adds bounds checking for `Integer` and `Long`.

*Feedback request*: To avoid repeating code, I have factored out a private top level function called `_validate_bounds`. This would make more sense as a method in a `Number` superclass that `Integer`, `Long` etc. inherit from. Since this is a major-ish change, it would be great to have feedback.

Remaining work:
- [x]  Bounds checking for casting traits, e.g. CLong
- [x]  Use `_validate_bounds` method in existing traitlets, e.g. `Float`. 






Patch:
diff --git a/traitlets/traitlets.py b/traitlets/traitlets.py
index e4a250f..99cefa9 100644
--- a/traitlets/traitlets.py
+++ b/traitlets/traitlets.py
@@ -250,7 +250,6 @@ def _validate_link(*tuples):
         if not trait_name in obj.traits():
             raise TypeError("%r has no trait %r" % (obj, trait_name))
 
-
 class link(object):
     """Link traits from different objects together so they remain in sync.
 
@@ -1821,6 +1820,30 @@ class Any(TraitType):
     info_text = 'any value'
 
 
+def _validate_bounds(trait, obj, value):
+    """
+    Validate that a number to be applied to a trait is between bounds.
+
+    If value is not between min_bound and max_bound, this raises a
+    TraitError with an error message appropriate for this trait.
+    """
+    if trait.min is not None and value < trait.min:
+        raise TraitError(
+            "The value of the '{name}' trait of {klass} instance should "
+            "not be less than {min_bound}, but a value of {value} was "
+            "specified".format(
+                name=trait.name, klass=class_of(obj),
+                value=value, min_bound=trait.min))
+    if trait.max is not None and value > trait.max:
+        raise TraitError(
+            "The value of the '{name}' trait of {klass} instance should "
+            "not be greater than {max_bound}, but a value of {value} was "
+            "specified".format(
+                name=trait.name, klass=class_of(obj),
+                value=value, max_bound=trait.max))
+    return value
+
+
 class Int(TraitType):
     """An int trait."""
 
@@ -1837,17 +1860,7 @@ class Int(TraitType):
     def validate(self, obj, value):
         if not isinstance(value, int):
             self.error(obj, value)
-        if self.max is not None and value > self.max:        
-            raise TraitError("The value of the '%s' trait of %s instance should "
-                             "not be greater than %s, but a value of %s was "
-                             "specified" % (self.name, class_of(obj),
-                                            self.max, value))
-        if self.min is not None and value < self.min:
-            raise TraitError("The value of the '%s' trait of %s instance should "
-                             "not be less than %s, but a value of %s was "
-                             "specified" % (self.name, class_of(obj), 
-                                            self.min, value))
-        return value
+        return _validate_bounds(self, obj, value)
 
 
 class CInt(Int):
@@ -1855,9 +1868,11 @@ class CInt(Int):
 
     def validate(self, obj, value):
         try:
-            return int(value)
+            value = int(value)
         except:
             self.error(obj, value)
+        return _validate_bounds(self, obj, value)
+
 
 if six.PY2:
     class Long(TraitType):
@@ -1866,22 +1881,35 @@ if six.PY2:
         default_value = 0
         info_text = 'a long'
 
-        def validate(self, obj, value):
+        def __init__(self, default_value=Undefined, allow_none=None, **kwargs):
+            self.min = kwargs.pop('min', None)
+            self.max = kwargs.pop('max', None)
+            super(Long, self).__init__(
+                default_value=default_value,
+                allow_none=allow_none, **kwargs)
+
+        def _validate_long(self, obj, value):
             if isinstance(value, long):
                 return value
             if isinstance(value, int):
                 return long(value)
             self.error(obj, value)
 
+        def validate(self, obj, value):
+            value = self._validate_long(obj, value)
+            return _validate_bounds(self, obj, value)
+
 
     class CLong(Long):
         """A casting version of the long integer trait."""
 
         def validate(self, obj, value):
             try:
-                return long(value)
+                value = long(value)
             except:
                 self.error(obj, value)
+            return _validate_bounds(self, obj, value)
+
 
     class Integer(TraitType):
         """An integer trait.
@@ -1891,7 +1919,14 @@ if six.PY2:
         default_value = 0
         info_text = 'an integer'
 
-        def validate(self, obj, value):
+        def __init__(self, default_value=Undefined, allow_none=None, **kwargs):
+            self.min = kwargs.pop('min', None)
+            self.max = kwargs.pop('max', None)
+            super(Integer, self).__init__(
+                default_value=default_value,
+                allow_none=allow_none, **kwargs)
+
+        def _validate_int(self, obj, value):
             if isinstance(value, int):
                 return value
             if isinstance(value, long):
@@ -1904,6 +1939,11 @@ if six.PY2:
                 if isinstance(value, Int64):
                     return int(value)
             self.error(obj, value)
+
+        def validate(self, obj, value):
+            value = self._validate_int(obj, value)
+            return _validate_bounds(self, obj, value)
+
 else:
     Long, CLong = Int, CInt
     Integer = Int
@@ -1919,7 +1959,7 @@ class Float(TraitType):
                  allow_none=None, **kwargs):
         self.min = kwargs.pop('min', -float('inf'))
         self.max = kwargs.pop('max', float('inf'))
-        super(Float, self).__init__(default_value=default_value, 
+        super(Float, self).__init__(default_value=default_value,
                                     allow_none=allow_none, **kwargs)
 
     def validate(self, obj, value):
@@ -1927,12 +1967,7 @@ class Float(TraitType):
             value = float(value)
         if not isinstance(value, float):
             self.error(obj, value)
-        if value > self.max or value < self.min:
-            raise TraitError("The value of the '%s' trait of %s instance should "
-                             "be between %s and %s, but a value of %s was "
-                             "specified" % (self.name, class_of(obj),
-                                            self.min, self.max, value))
-        return value
+        return _validate_bounds(self, obj, value)
 
 
 class CFloat(Float):
@@ -1940,9 +1975,11 @@ class CFloat(Float):
 
     def validate(self, obj, value):
         try:
-            return float(value)
+            value = float(value)
         except:
             self.error(obj, value)
+        return _validate_bounds(self, obj, value)
+
 
 class Complex(TraitType):
     """A trait for complex numbers."""


Test patch:
diff --git a/traitlets/tests/test_traitlets.py b/traitlets/tests/test_traitlets.py
index 46e7574..3ae2b68 100644
--- a/traitlets/tests/test_traitlets.py
+++ b/traitlets/tests/test_traitlets.py
@@ -18,9 +18,9 @@ from pytest import mark
 
 from traitlets import (
     HasTraits, MetaHasTraits, TraitType, Any, Bool, CBytes, Dict, Enum,
-    Int, Long, Integer, Float, Complex, Bytes, Unicode, TraitError,
-    Union, All, Undefined, Type, This, Instance, TCPAddress, List, Tuple,
-    ObjectName, DottedObjectName, CRegExp, link, directional_link,
+    Int, CInt, Long, CLong, Integer, Float, CFloat, Complex, Bytes, Unicode,
+    TraitError, Union, All, Undefined, Type, This, Instance, TCPAddress,
+    List, Tuple, ObjectName, DottedObjectName, CRegExp, link, directional_link,
     ForwardDeclaredType, ForwardDeclaredInstance, validate, observe, default,
     observe_compat, BaseDescriptor, HasDescriptors,
 )
@@ -1148,6 +1148,32 @@ class TestInt(TraitTestBase):
         _bad_values.extend([long(10), long(-10), 10*sys.maxint, -10*sys.maxint])
 
 
+class CIntTrait(HasTraits):
+    value = CInt('5')
+
+class TestCInt(TraitTestBase):
+    obj = CIntTrait()
+
+    _default_value = 5
+    _good_values   = ['10', '-10', u'10', u'-10', 10, 10.0, -10.0, 10.1]
+    _bad_values    = ['ten', u'ten', [10], {'ten': 10},(10,),
+                      None, 1j, '10.1', u'10.1']
+
+    def coerce(self, n):
+        return int(n)
+
+
+class MinBoundCIntTrait(HasTraits):
+    value = CInt('5', min=3)
+
+class TestMinBoundCInt(TestCInt):
+    obj = MinBoundCIntTrait()
+
+    _default_value = 5
+    _good_values   = [3, 3.0, '3']
+    _bad_values    = [2.6, 2, -3, -3.0]
+
+
 class LongTrait(HasTraits):
 
     value = Long(99 if six.PY3 else long(99))
@@ -1174,6 +1200,54 @@ class TestLong(TraitTestBase):
         self.assertEqual(type(self.obj.value), long)
 
 
+class MinBoundLongTrait(HasTraits):
+    value = Long(99 if six.PY3 else long(99), min=5)
+
+class TestMinBoundLong(TraitTestBase):
+    obj = MinBoundLongTrait()
+
+    _default_value = 99 if six.PY3 else long(99)
+    _good_values   = [5, 10]
+    _bad_values    = [4, -10]
+
+
+class MaxBoundLongTrait(HasTraits):
+    value = Long(5 if six.PY3 else long(5), max=10)
+
+class TestMaxBoundLong(TraitTestBase):
+    obj = MaxBoundLongTrait()
+
+    _default_value = 5 if six.PY3 else long(5)
+    _good_values   = [10, -2]
+    _bad_values    = [11, 20]
+
+
+class CLongTrait(HasTraits):
+    value = CLong('5')
+
+class TestCLong(TraitTestBase):
+    obj = CLongTrait()
+
+    _default_value = 5 if six.PY3 else long(5)
+    _good_values   = ['10', '-10', u'10', u'-10', 10, 10.0, -10.0, 10.1]
+    _bad_values    = ['ten', u'ten', [10], {'ten': 10},(10,),
+                      None, 1j, '10.1', u'10.1']
+
+    def coerce(self, n):
+        return int(n) if six.PY3 else long(n)
+
+
+class MaxBoundCLongTrait(HasTraits):
+    value = CLong('5', max=10)
+
+class TestMaxBoundCLong(TestCLong):
+    obj = MaxBoundCLongTrait()
+
+    _default_value = 5 if six.PY3 else long(5)
+    _good_values   = [10, '10', 10.3]
+    _bad_values    = [11.0, '11']
+
+
 class IntegerTrait(HasTraits):
     value = Integer(1)
 
@@ -1192,6 +1266,28 @@ class TestInteger(TestLong):
         self.assertEqual(type(self.obj.value), int)
 
 
+class MinBoundIntegerTrait(HasTraits):
+    value = Integer(5, min=3)
+
+class TestMinBoundInteger(TraitTestBase):
+    obj = MinBoundIntegerTrait()
+
+    _default_value = 5
+    _good_values   = 3, 20
+    _bad_values    = [2, -10]
+
+
+class MaxBoundIntegerTrait(HasTraits):
+    value = Integer(1, max=3)
+
+class TestMaxBoundInteger(TraitTestBase):
+    obj = MaxBoundIntegerTrait()
+
+    _default_value = 1
+    _good_values   = 3, -2
+    _bad_values    = [4, 10]
+
+
 class FloatTrait(HasTraits):
 
     value = Float(99.0, max=200.0)
@@ -1209,6 +1305,23 @@ class TestFloat(TraitTestBase):
         _bad_values.extend([long(10), long(-10)])
 
 
+class CFloatTrait(HasTraits):
+
+    value = CFloat('99.0', max=200.0)
+
+class TestCFloat(TraitTestBase):
+
+    obj = CFloatTrait()
+
+    _default_value = 99.0
+    _good_values   = [10, 10.0, 10.5, '10.0', '10', '-10', '10.0', u'10']
+    _bad_values    = ['ten', u'ten', [10], {'ten': 10}, (10,), None, 1j,
+                      200.1, '200.1']
+
+    def coerce(self, v):
+        return float(v)
+
+
 class ComplexTrait(HasTraits):
 
     value = Complex(99.0-99.0j)