Merge pull request #393 from enthought/fix/consistent-floats

mdickinson · web-flow · commit 2e788165a19c · 2018-12-17T15:59:27.000Z
Clean up Float and BaseFloat validation
diff --git a/traits/ctraits.c b/traits/ctraits.c
@@ -3318,13 +3318,52 @@ validate_trait_integer ( trait_object * trait, has_traits_object * obj,
     return raise_trait_error( trait, obj, name, value );
 }
 
+
+/*-----------------------------------------------------------------------------
+|  Verifies that a Python value is convertible to float
+|
+|  Will convert anything whose type has a __float__ method to a Python
+|  float. Returns a Python object of exact type "float". Raises TraitError
+|  with a suitable message if the given value isn't convertible to float.
+|
+|  Any exception other than TypeError raised by the value's __float__ method
+|  will be propagated. A TypeError will be caught and turned into TraitError.
+|
++----------------------------------------------------------------------------*/
+
+static PyObject *
+validate_trait_float(trait_object * trait, has_traits_object * obj,
+                     PyObject * name, PyObject * value) {
+    /* Fast path for the most common case. */
+    if (PyFloat_CheckExact(value)) {
+        Py_INCREF(value);
+        return value;
+    }
+    else {
+        double value_as_double = PyFloat_AsDouble(value);
+        /* Translate a TypeError to a TraitError, but propagate
+           other exceptions. */
+        if (value_as_double == -1.0 && PyErr_Occurred()) {
+            if (PyErr_ExceptionMatches(PyExc_TypeError)) {
+                PyErr_Clear();
+                goto error;
+            }
+            return NULL;
+        }
+        return PyFloat_FromDouble(value_as_double);
+    }
+
+  error:
+    return raise_trait_error(trait, obj, name, value);
+}
+
 /*-----------------------------------------------------------------------------
 |  Verifies a Python value is a float within a specified range:
 +----------------------------------------------------------------------------*/
 
 static PyObject *
-validate_trait_float ( trait_object * trait, has_traits_object * obj,
-                       PyObject * name, PyObject * value ) {
+validate_trait_float_range ( trait_object * trait, has_traits_object * obj,
+                             PyObject * name, PyObject * value ) {
 
     register PyObject * low;
     register PyObject * high;
@@ -4015,7 +4054,7 @@ validate_trait_complex ( trait_object * trait, has_traits_object * obj,
                     goto done;
                 break;
 
-            case 20:  /* Integer check: */
+           case 20:  /* Integer check: */
 
                 /* Fast paths for the most common cases. */
 #if PY_MAJOR_VERSION < 3
@@ -4063,6 +4102,29 @@ validate_trait_complex ( trait_object * trait, has_traits_object * obj,
                 Py_DECREF(int_value);
                 return result;
 
+           case 21:  /* Float check */
+               /* Fast path for most common case. */
+               if (PyFloat_CheckExact(value)) {
+                   Py_INCREF(value);
+                   return value;
+               }
+               else {
+                   double value_as_double = PyFloat_AsDouble(value);
+                   if (value_as_double == -1.0 && PyErr_Occurred()) {
+                       /* TypeError indicates that we don't have a match;
+                          clear the error and continue with the next item
+                          in the complex sequence. */
+                       if (PyErr_ExceptionMatches(PyExc_TypeError)) {
+                           PyErr_Clear();
+                           break;
+                       }
+                       /* Any other exception is unexpected and likely
+                          a code bug; propagate it. */
+                       return NULL;
+                   }
+                   return PyFloat_FromDouble(value_as_double);
+               }
+
             default:  /* Should never happen...indicates an internal error: */
                 goto error;
         }
@@ -4086,7 +4148,7 @@ static trait_validate validate_handlers[] = {
 #else
     validate_trait_self_type,   NULL,
 #endif // #if PY_MAJOR_VERSION < 3
-    validate_trait_float,       validate_trait_enum,
+    validate_trait_float_range, validate_trait_enum,
     validate_trait_map,         validate_trait_complex,
     NULL,                       validate_trait_tuple,
     validate_trait_prefix_map,  validate_trait_coerce_type,
@@ -4097,6 +4159,7 @@ static trait_validate validate_handlers[] = {
     setattr_validate2,           setattr_validate3,
 /*  ...End of __getstate__ method entries */
     validate_trait_adapt,        validate_trait_integer,
+    validate_trait_float,
 };
 
 static PyObject *
@@ -4246,6 +4309,12 @@ _trait_set_validate ( trait_object * trait, PyObject * args ) {
                     if ( n == 1 )
                         goto done;
                     break;
+
+                case 21:  /* Float check: */
+                    if ( n == 1 )
+                        goto done;
+                    break;
+
             }
         }
     }
diff --git a/traits/tests/test_float.py b/traits/tests/test_float.py
@@ -17,18 +17,48 @@
 """
 import sys
 
+try:
+    import numpy
+except ImportError:
+    numpy_available = False
+else:
+    numpy_available = True
+
 from traits.testing.unittest_tools import unittest
 
-from ..api import BaseFloat, Float, HasTraits
+from ..api import BaseFloat, Either, Float, HasTraits, TraitError, Unicode
+
+
+class MyFloat(object):
+    def __init__(self, value):
+        self._value = value
+
+    def __float__(self):
+        return self._value
+
+
+class BadFloat(object):
+    def __float__(self):
+        raise ZeroDivisionError
 
 
 class FloatModel(HasTraits):
     value = Float
 
+    # Assignment to the `Either` trait exercises a different C code path (see
+    # validate_trait_complex in ctraits.c).
+    value_or_none = Either(None, Float)
+
+    float_or_text = Either(Float, Unicode)
+
 
 class BaseFloatModel(HasTraits):
     value = BaseFloat
 
+    value_or_none = Either(None, BaseFloat)
+
+    float_or_text = Either(Float, Unicode)
+
 
 class CommonFloatTests(object):
     """ Common tests for Float and BaseFloat """
@@ -38,36 +68,110 @@ def test_default(self):
 
     def test_accepts_float(self):
         a = self.test_class()
+
         a.value = 5.6
         self.assertIs(type(a.value), float)
         self.assertEqual(a.value, 5.6)
 
+        a.value_or_none = 5.6
+        self.assertIs(type(a.value_or_none), float)
+        self.assertEqual(a.value_or_none, 5.6)
+
     def test_accepts_int(self):
         a = self.test_class()
+
         a.value = 2
         self.assertIs(type(a.value), float)
         self.assertEqual(a.value, 2.0)
 
+        a.value_or_none = 2
+        self.assertIs(type(a.value_or_none), float)
+        self.assertEqual(a.value_or_none, 2.0)
+
+    def test_accepts_float_like(self):
+        a = self.test_class()
+
+        a.value = MyFloat(1729.0)
+        self.assertIs(type(a.value), float)
+        self.assertEqual(a.value, 1729.0)
+
+        a.value = MyFloat(594.0)
+        self.assertIs(type(a.value), float)
+        self.assertEqual(a.value, 594.0)
+
+    def test_rejects_string(self):
+        a = self.test_class()
+        with self.assertRaises(TraitError):
+            a.value = "2.3"
+        with self.assertRaises(TraitError):
+            a.value_or_none = "2.3"
+
+    def test_bad_float_exceptions_propagated(self):
+        a = self.test_class()
+        with self.assertRaises(ZeroDivisionError):
+            a.value = BadFloat()
+
+    def test_compound_trait_float_conversion_fail(self):
+        # Check that a failure to convert to float doesn't terminate
+        # an assignment to a compound trait.
+        a = self.test_class()
+        a.float_or_text = u"not a float"
+        self.assertEqual(a.float_or_text, u"not a float")
+
     @unittest.skipUnless(sys.version_info < (3,), "Not applicable to Python 3")
     def test_accepts_small_long(self):
         a = self.test_class()
+
         a.value = long(2)
         self.assertIs(type(a.value), float)
         self.assertEqual(a.value, 2.0)
 
+        a.value_or_none = long(2)
+        self.assertIs(type(a.value_or_none), float)
+        self.assertEqual(a.value_or_none, 2.0)
+
     @unittest.skipUnless(sys.version_info < (3,), "Not applicable to Python 3")
     def test_accepts_large_long(self):
         a = self.test_class()
+
         # Value large enough to be a long on Python 2.
         a.value = 2**64
         self.assertIs(type(a.value), float)
         self.assertEqual(a.value, 2**64)
 
+        a.value_or_none = 2**64
+        self.assertIs(type(a.value_or_none), float)
+        self.assertEqual(a.value_or_none, 2**64)
+
+    @unittest.skipUnless(numpy_available, "Test requires NumPy")
+    def test_accepts_numpy_floats(self):
+        test_values = [
+            numpy.float64(2.3),
+            numpy.float32(3.7),
+            numpy.float16(1.28),
+        ]
+        a = self.test_class()
+        for test_value in test_values:
+            a.value = test_value
+            self.assertIs(type(a.value), float)
+            self.assertEqual(a.value, test_value)
+
+            a.value_or_none = test_value
+            self.assertIs(type(a.value_or_none), float)
+            self.assertEqual(a.value_or_none, test_value)
+
 
 class TestFloat(unittest.TestCase, CommonFloatTests):
     def setUp(self):
         self.test_class = FloatModel
 
+    def test_exceptions_propagate_in_compound_trait(self):
+        # This test doesn't currently pass for BaseFloat, which is why it's not
+        # in the common tests. That's probably a bug.
+        a = self.test_class()
+        with self.assertRaises(ZeroDivisionError):
+            a.value_or_none = BadFloat()
+
 
 class TestBaseFloat(unittest.TestCase, CommonFloatTests):
     def setUp(self):
diff --git a/traits/trait_types.py b/traits/trait_types.py
@@ -123,6 +123,23 @@ def default_text_editor ( trait, type = None ):
                        enter_set = enter_set,
                        evaluate  = type )
 
+
+# Generic validators
+
+def _validate_float(value):
+    """
+    Convert an arbitrary Python object to a float, or raise TypeError.
+    """
+    if type(value) == float:  # fast path for common case
+        return value
+    try:
+        nb_float = type(value).__float__
+    except AttributeError:
+        raise TypeError(
+            "Object of type {!r} not convertible to float".format(type(value)))
+    return nb_float(value)
+
+
 #-------------------------------------------------------------------------------
 #  'Any' trait:
 #-------------------------------------------------------------------------------
@@ -241,6 +258,7 @@ class Long ( BaseLong ):
     #: The C-level fast validator to use:
     fast_validate = long_fast_validate
 
+
 #-------------------------------------------------------------------------------
 #  'BaseFloat' and 'Float' traits:
 #-------------------------------------------------------------------------------
@@ -262,13 +280,10 @@ def validate ( self, object, name, value ):
 
             Note: The 'fast validator' version performs this check in C.
         """
-        if isinstance( value, float ):
-            return value
-
-        if isinstance( value, ( int, long ) ):
-            return float( value )
-
-        self.error( object, name, value )
+        try:
+            return _validate_float(value)
+        except TypeError:
+            self.error(object, name, value)
 
     def create_editor ( self ):
         """ Returns the default traits UI editor for this type of trait.
@@ -282,7 +297,7 @@ class Float ( BaseFloat ):
     """
 
     #: The C-level fast validator to use:
-    fast_validate = float_fast_validate
+    fast_validate = ( 21, )
 
 #-------------------------------------------------------------------------------
 #  'BaseComplex' and 'Complex' traits:
