core: Avoid ambiguity with numeric type conversion

include/eigenpy/variant.hpp

@@ -36,32 +36,6 @@ struct empty_variant {};
 template <typename T>
 struct is_empty_variant : std::false_type {};
 
-/// Convert None to a {boost,std}::variant with boost::blank or std::monostate
-/// value
-template <typename Variant>
-struct EmptyConvertible {
-  static void registration() {
-    bp::converter::registry::push_back(convertible, construct,
-                                       bp::type_id<Variant>());
-  }
-
-  // convertible only for None
-  static void* convertible(PyObject* obj) {
-    return (obj == Py_None) ? obj : nullptr;
-  };
-
-  // construct in place
-  static void construct(PyObject*,
-                        bp::converter::rvalue_from_python_stage1_data* data) {
-    void* storage =
-        reinterpret_cast<bp::converter::rvalue_from_python_storage<Variant>*>(
-            data)
-            ->storage.bytes;
-    new (storage) Variant(typename empty_variant<Variant>::type());
-    data->convertible = storage;
-  };
-};
-
 #ifdef EIGENPY_WITH_CXX17_SUPPORT
 
 /// std::variant implementation
@@ -126,6 +100,90 @@ struct empty_variant<boost::variant<Alternatives...> > {
 template <>
 struct is_empty_variant<boost::blank> : std::true_type {};
 
+/// Convert None to a {boost,std}::variant with boost::blank or std::monostate
+/// value
+template <typename Variant>
+struct EmptyConvertible {
+  static void registration() {
+    bp::converter::registry::push_back(convertible, construct,
+                                       bp::type_id<Variant>());
+  }
+
+  // convertible only for None
+  static void* convertible(PyObject* obj) {
+    return (obj == Py_None) ? obj : nullptr;
+  };
+
+  // construct in place
+  static void construct(PyObject*,
+                        bp::converter::rvalue_from_python_stage1_data* data) {
+    void* storage =
+        reinterpret_cast<bp::converter::rvalue_from_python_storage<Variant>*>(
+            data)
+            ->storage.bytes;
+    new (storage) Variant(typename empty_variant<Variant>::type());
+    data->convertible = storage;
+  };
+};
+
+/// Implement convertible and expected_pytype for bool, integer and float
+template <typename T, class Enable = void>
+struct NumericConvertibleImpl {};
+
+template <typename T>
+struct NumericConvertibleImpl<
+    T, typename std::enable_if<std::is_same<T, bool>::value>::type> {
+  static void* convertible(PyObject* obj) {
+    return PyBool_Check(obj) ? obj : nullptr;
+  }
+
+  static PyTypeObject const* expected_pytype() { return &PyBool_Type; }
+};
+
+template <typename T>
+struct NumericConvertibleImpl<
+    T, typename std::enable_if<!std::is_same<T, bool>::value &&
+                               std::is_integral<T>::value>::type> {
+  static void* convertible(PyObject* obj) {
+    // PyLong return true for bool type
+    return (PyLong_Check(obj) && !PyBool_Check(obj)) ? obj : nullptr;
+  }
+
+  static PyTypeObject const* expected_pytype() { return &PyLong_Type; }
+};
+
+template <typename T>
+struct NumericConvertibleImpl<
+    T, typename std::enable_if<std::is_floating_point<T>::value>::type> {
+  static void* convertible(PyObject* obj) {
+    return PyFloat_Check(obj) ? obj : nullptr;
+  }
+
+  static PyTypeObject const* expected_pytype() { return &PyFloat_Type; }
+};
+
+/// Convert numeric type to Variant without ambiguity
+template <typename T, typename Variant>
+struct NumericConvertible {
+  static void registration() {
+    bp::converter::registry::push_back(
+        &convertible, &bp::converter::implicit<T, Variant>::construct,
+        bp::type_id<Variant>()
+#ifndef BOOST_PYTHON_NO_PY_SIGNATURES
+            ,
+        &expected_pytype
+#endif
+    );
+  }
+
+  static void* convertible(PyObject* obj) {
+    return NumericConvertibleImpl<T>::convertible(obj);
+  }
+  static PyTypeObject const* expected_pytype() {
+    return NumericConvertibleImpl<T>::expected_pytype();
+  }
+};
+
 /// Convert {boost,std}::variant<class...> alternative to a Python object.
 /// This converter copy the alternative.
 template <typename Variant>
@@ -225,7 +283,15 @@ struct VariantConvertible {
     EmptyConvertible<variant_type>::registration();
   }
 
-  template <class T, typename std::enable_if<!is_empty_variant<T>::value,
+  template <class T, typename std::enable_if<!is_empty_variant<T>::value &&
+                                                 std::is_arithmetic<T>::value,
+                                             bool>::type = true>
+  void operator()(T) {
+    NumericConvertible<T, variant_type>::registration();
+  }
+
+  template <class T, typename std::enable_if<!is_empty_variant<T>::value &&
+                                                 !std::is_arithmetic<T>::value,
                                              bool>::type = true>
   void operator()(T) {
     bp::implicitly_convertible<T, variant_type>();

unittest/python/test_variant.py.in

@@ -4,13 +4,9 @@ variant_module = importlib.import_module("@MODNAME@")
 V1 = variant_module.V1
 V2 = variant_module.V2
 VariantHolder = variant_module.VariantHolder
-VariantNoneHolder = variant_module.VariantNoneHolder
-VariantArithmeticHolder = variant_module.VariantArithmeticHolder
-VariantBoolHolder = variant_module.VariantBoolHolder
+VariantFullHolder = variant_module.VariantFullHolder
 make_variant = variant_module.make_variant
-make_variant_none = variant_module.make_variant_none
-make_variant_arithmetic = variant_module.make_variant_arithmetic
-make_variant_bool = variant_module.make_variant_bool
+make_variant_full = variant_module.make_variant_full
 
 variant = make_variant()
 assert isinstance(variant, V1)
@@ -48,48 +44,40 @@ assert isinstance(variant_holder.variant, V2)
 assert variant_holder.variant.v == v2.v
 
 # Test variant that hold a None value
-v_none = make_variant_none()
-assert v_none is None
+v_full = make_variant_full()
+assert v_full is None
+
+variant_full_holder = VariantFullHolder()
 
-variant_none_holder = VariantNoneHolder()
-v_none = variant_none_holder.variant
+# Test None
+v_none = variant_full_holder.variant
+assert v_none is None
+variant_full_holder.variant = None
 assert v_none is None
 
+# Test V1
 v1 = V1()
-v1.v = 1
-variant_none_holder.variant = v1
-assert variant_none_holder.variant.v == 1
-v1 = variant_none_holder.variant
 v1.v = 10
-assert variant_none_holder.variant.v == 10
-variant_none_holder.variant = None
-
-
-# Test variant that hold base type
-v_arithmetic = make_variant_arithmetic()
-assert isinstance(v_arithmetic, int)
-
-variant_arithmetic_holder = VariantArithmeticHolder()
-assert isinstance(variant_arithmetic_holder.variant, int)
-variant_arithmetic_holder.variant = 2
-# Raise an exception if return_internal_postcall is called
-assert variant_arithmetic_holder.variant == 2
-
-variant_arithmetic_holder.variant = 2.0
-assert isinstance(variant_arithmetic_holder.variant, float)
-# Raise an exception if return_internal_postcall is called
-assert variant_arithmetic_holder.variant == 2.0
-
-v_bool = make_variant_bool()
-assert isinstance(v_bool, bool)
-
-variant_bool_holder = VariantBoolHolder()
-assert isinstance(variant_bool_holder.variant, bool)
-variant_bool_holder.variant = False
-# Raise an exception if return_internal_postcall is called
-assert not variant_bool_holder.variant
-
-variant_bool_holder.variant = 2.0
-assert isinstance(variant_bool_holder.variant, float)
-# Raise an exception if return_internal_postcall is called
-assert variant_bool_holder.variant == 2.0
+variant_full_holder.variant = v1
+assert variant_full_holder.variant.v == 10
+assert isinstance(variant_full_holder.variant, V1)
+# Test V1 ref
+v1 = variant_full_holder.variant
+v1.v = 100
+assert variant_full_holder.variant.v == 100
+variant_full_holder.variant = None
+
+# Test bool
+variant_full_holder.variant = True
+assert variant_full_holder.variant
+assert isinstance(variant_full_holder.variant, bool)
+
+# Test int
+variant_full_holder.variant = 3
+assert variant_full_holder.variant == 3
+assert isinstance(variant_full_holder.variant, int)
+
+# Test float
+variant_full_holder.variant = 3.14
+assert variant_full_holder.variant == 3.14
+assert isinstance(variant_full_holder.variant, float)

unittest/variant.cpp.in

@@ -32,35 +32,19 @@ struct MyVariantNoneHelper<std::variant<Alternatives...> > {
 };
 #endif
 
-typedef typename MyVariantNoneHelper<VARIANT<V1> >::type MyVariantNone;
-
-typedef VARIANT<int, double> MyVariantArithmetic;
-
-// There is a conversion conflict between int and bool
-typedef VARIANT<bool, double> MyVariantBool;
+typedef typename MyVariantNoneHelper<VARIANT<V1, bool, int, double> >::type
+    MyVariantFull;
 
 MyVariant make_variant() { return V1(); }
 
-MyVariantNone make_variant_none() { return MyVariantNone(); }
-
-MyVariantArithmetic make_variant_arithmetic() { return MyVariantArithmetic(); }
-
-MyVariantBool make_variant_bool() { return MyVariantBool(); }
+MyVariantFull make_variant_full() { return MyVariantFull(); }
 
 struct VariantHolder {
   MyVariant variant;
 };
 
-struct VariantNoneHolder {
-  MyVariantNone variant;
-};
-
-struct VariantArithmeticHolder {
-  MyVariantArithmetic variant;
-};
-
-struct VariantBoolHolder {
-  MyVariantBool variant;
+struct VariantFullHolder {
+  MyVariantFull variant;
 };
 
 BOOST_PYTHON_MODULE(@MODNAME@) {
@@ -82,37 +66,13 @@ BOOST_PYTHON_MODULE(@MODNAME@) {
                                     Converter::return_internal_reference()),
                     bp::make_setter(&VariantHolder::variant));
 
-  typedef eigenpy::VariantConverter<MyVariantNone> ConverterNone;
-  ConverterNone::registration();
-  bp::def("make_variant_none", make_variant_none);
+  typedef eigenpy::VariantConverter<MyVariantFull> ConverterFull;
+  ConverterFull::registration();
+  bp::def("make_variant_full", make_variant_full);
 
-  boost::python::class_<VariantNoneHolder>("VariantNoneHolder", bp::init<>())
+  boost::python::class_<VariantFullHolder>("VariantFullHolder", bp::init<>())
       .add_property("variant",
-                    bp::make_getter(&VariantNoneHolder::variant,
-                                    ConverterNone::return_internal_reference()),
-                    bp::make_setter(&VariantNoneHolder::variant));
-
-  typedef eigenpy::VariantConverter<MyVariantArithmetic> ConverterArithmetic;
-  ConverterArithmetic::registration();
-  bp::def("make_variant_arithmetic", make_variant_arithmetic);
-
-  boost::python::class_<VariantArithmeticHolder>("VariantArithmeticHolder",
-                                                 bp::init<>())
-      .add_property(
-          "variant",
-          bp::make_getter(&VariantArithmeticHolder::variant,
-                          ConverterArithmetic::return_internal_reference()),
-          bp::make_setter(&VariantArithmeticHolder::variant));
-
-  typedef eigenpy::VariantConverter<MyVariantBool> ConverterBool;
-  ConverterBool::registration();
-  bp::def("make_variant_bool", make_variant_bool);
-
-  boost::python::class_<VariantBoolHolder>("VariantBoolHolder",
-                                                 bp::init<>())
-      .add_property(
-          "variant",
-          bp::make_getter(&VariantBoolHolder::variant,
-                          ConverterBool::return_internal_reference()),
-          bp::make_setter(&VariantBoolHolder::variant));
+                    bp::make_getter(&VariantFullHolder::variant,
+                                    ConverterFull::return_internal_reference()),
+                    bp::make_setter(&VariantFullHolder::variant));
 }