script_interface: Improve support for dicts

Add support for dicts with string keys. Improve error messages
for failed conversions by get_value() and reduce complexity of
demangled symbols by removing allocators from container types.

src/python/espressomd/script_interface.pxd

@@ -17,7 +17,6 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 
-from libcpp.map cimport map
 from libcpp.unordered_map cimport unordered_map
 from libcpp.vector cimport vector
 from libcpp.string cimport string
@@ -47,8 +46,6 @@ cdef extern from "script_interface/ScriptInterface.hpp" namespace "ScriptInterfa
         Variant get_parameter(const string & name) except +
         void set_parameter(const string & name, const Variant & value) except +
         Variant call_method(const string & name, const VariantMap & parameters) except +
-        void set_state(map[string, Variant]) except +
-        map[string, Variant] get_state() except +
         string_ref name()
 
 cdef extern from "script_interface/ContextManager.hpp" namespace "ScriptInterface::ContextManager":

src/python/espressomd/script_interface.pyx

@@ -20,6 +20,7 @@ from .utils cimport Vector2d, Vector3d, Vector4d, make_array_locked
 cimport cpython.object
 
 from libcpp.memory cimport shared_ptr, make_shared
+from libcpp.utility cimport pair
 
 cdef shared_ptr[ContextManager] _om
 
@@ -189,7 +190,8 @@ cdef Variant python_object_to_variant(value) except *:
     """Convert Python objects to C++ Variant objects."""
 
     cdef vector[Variant] vec
-    cdef unordered_map[int, Variant] vmap
+    cdef unordered_map[int, Variant] map_int2var
+    cdef unordered_map[string, Variant] map_str2var
     cdef PObjectRef oref
 
     if value is None:
@@ -202,20 +204,28 @@ cdef Variant python_object_to_variant(value) except *:
         oref = value.get_sip()
         return make_variant(oref.sip)
     elif isinstance(value, dict):
+        if all(map(lambda x: isinstance(x, (int, np.integer)), value.keys())):
+            for key, value in value.items():
+                map_int2var[int(key)] = python_object_to_variant(value)
+            return make_variant[unordered_map[int, Variant]](map_int2var)
+        elif all(map(lambda x: isinstance(x, (str, np.str_)), value.keys())):
+            for key, value in value.items():
+                map_str2var[utils.to_char_pointer(
+                    str(key))] = python_object_to_variant(value)
+            return make_variant[unordered_map[string, Variant]](map_str2var)
         for k, v in value.items():
-            if not isinstance(k, int):
+            if not isinstance(k, (str, int, np.integer, np.str_)):
                 raise TypeError(
                     f"No conversion from type "
                     f"'dict_item([({type(k).__name__}, {type(v).__name__})])'"
-                    f" to 'Variant[std::unordered_map<int, Variant>]'")
-            vmap[k] = python_object_to_variant(v)
-        return make_variant[unordered_map[int, Variant]](vmap)
-    elif hasattr(value, '__iter__') and type(value) != str:
+                    f" to 'Variant[std::unordered_map<int, Variant>]' or"
+                    f" to 'Variant[std::unordered_map<std::string, Variant>]'")
+    elif type(value) in (str, np.str_):
+        return make_variant[string](utils.to_char_pointer(str(value)))
+    elif hasattr(value, '__iter__'):
         for e in value:
             vec.push_back(python_object_to_variant(e))
         return make_variant[vector[Variant]](vec)
-    elif type(value) == str:
-        return make_variant[string](utils.to_char_pointer(value))
     elif type(value) == type(True):
         return make_variant[bool](value)
     elif np.issubdtype(np.dtype(type(value)), np.signedinteger):
@@ -230,7 +240,10 @@ cdef variant_to_python_object(const Variant & value) except +:
     """Convert C++ Variant objects to Python objects."""
 
     cdef vector[Variant] vec
-    cdef unordered_map[int, Variant] vmap
+    cdef unordered_map[int, Variant] map_int2var
+    cdef unordered_map[string, Variant] map_str2var
+    cdef pair[int, Variant] pair_int2var
+    cdef pair[string, Variant] pair_str2var
     cdef shared_ptr[ObjectHandle] ptr
     cdef Vector2d vec2d
     cdef Vector4d vec4d
@@ -288,12 +301,24 @@ cdef variant_to_python_object(const Variant & value) except +:
             res.append(variant_to_python_object(i))
 
         return res
+
     if is_type[unordered_map[int, Variant]](value):
-        vmap = get_value[unordered_map[int, Variant]](value)
+        map_int2var = get_value[unordered_map[int, Variant]](value)
+        res = {}
+
+        for pair_int2var in map_int2var:
+            res[pair_int2var.first] = variant_to_python_object(
+                pair_int2var.second)
+
+        return res
+
+    if is_type[unordered_map[string, Variant]](value):
+        map_str2var = get_value[unordered_map[string, Variant]](value)
         res = {}
 
-        for kv in vmap:
-            res[kv.first] = variant_to_python_object(kv.second)
+        for pair_str2var in map_str2var:
+            res[utils.to_str(pair_str2var.first)] = variant_to_python_object(
+                pair_str2var.second)
 
         return res
 

src/script_interface/Variant.hpp

@@ -71,7 +71,8 @@ using Variant = boost::make_recursive_variant<
     None, bool, int, std::size_t, double, std::string, ObjectRef,
     Utils::Vector2d, Utils::Vector3d, Utils::Vector4d, std::vector<int>,
     std::vector<double>, std::vector<boost::recursive_variant_>,
-    std::unordered_map<int, boost::recursive_variant_>>::type;
+    std::unordered_map<int, boost::recursive_variant_>,
+    std::unordered_map<std::string, boost::recursive_variant_>>::type;
 
 using VariantMap = std::unordered_map<std::string, Variant>;
 

src/script_interface/get_value.hpp

@@ -40,62 +40,87 @@ namespace ScriptInterface {
 namespace detail {
 
 /**
- * @brief Demangle the symbol of a container @c value_type or @c mapped_type.
- * Only for recursive variant container types used in @ref Variant.
- * Other container types and non-container objects return an empty string.
+ * @brief Convert a demangled symbol into a human-readable name that omits
+ * container allocators, key hashes and implementation-specific namespaces.
+ * When the data type involves the @ref Variant type, it is recursively
+ * replaced by the string "ScriptInterface::Variant".
  */
-template <typename Container> struct demangle_container_value_type {
-private:
-  template <typename T> struct is_std_vector : std::false_type {};
-  template <typename... Args>
-  struct is_std_vector<std::vector<Args...>> : std::true_type {};
-
-  template <typename T> struct is_std_unordered_map : std::false_type {};
-  template <typename... Args>
-  struct is_std_unordered_map<std::unordered_map<Args...>> : std::true_type {};
-
-public:
-  template <typename T = Container,
-            std::enable_if_t<!is_std_vector<T>::value and
-                                 !is_std_unordered_map<T>::value,
-                             bool> = true>
-  auto operator()() const {
-    return std::string("");
+namespace demangle {
+
+/** @brief Simplify the demangled symbol of an object. */
+template <typename T> auto simplify_symbol(T const *) {
+  auto constexpr is_string = std::is_same<T, std::string>::value;
+  auto const symbol_for_variant = Utils::demangle<Variant>();
+  auto const name_for_variant = std::string("ScriptInterface::Variant");
+  auto name = (is_string) ? std::string{"std::string"} : Utils::demangle<T>();
+  for (std::string::size_type pos{};
+       (pos = name.find(symbol_for_variant, pos)) != name.npos;
+       pos += name_for_variant.length()) {
+    name.replace(pos, symbol_for_variant.length(), name_for_variant);
   }
+  return name;
+}
 
-  template <typename T = Container,
-            std::enable_if_t<is_std_vector<T>::value, bool> = true>
-  auto operator()() const {
-    return Utils::demangle<typename T::value_type>();
-  }
+/** @overload */
+template <typename T> auto simplify_symbol(std::vector<T> const *) {
+  auto const name_val = simplify_symbol(static_cast<T *>(nullptr));
+  return "std::vector<" + name_val + ">";
+}
 
-  template <typename T = Container,
-            std::enable_if_t<is_std_unordered_map<T>::value, bool> = true>
-  auto operator()() const {
-    return Utils::demangle<typename T::mapped_type>();
+/** @overload */
+template <typename K, typename V>
+auto simplify_symbol(std::unordered_map<K, V> const *) {
+  auto const name_key = simplify_symbol(static_cast<K *>(nullptr));
+  auto const name_val = simplify_symbol(static_cast<V *>(nullptr));
+  return "std::unordered_map<" + name_key + ", " + name_val + ">";
+}
+
+struct simplify_symbol_visitor : boost::static_visitor<std::string> {
+  template <class T> std::string operator()(const T &) const {
+    return simplify_symbol(static_cast<T *>(nullptr));
   }
 };
 
-/**
- * @brief Demangle the data type of an object wrapped inside a @ref Variant.
- * When the data type involves a recursive variant (e.g. containers), the
- * demangled symbol name is processed to replace all occurences of the
- * variant symbol name by a human-readable string "ScriptInterface::Variant".
- */
-struct type_label_visitor : boost::static_visitor<std::string> {
+/** @brief Simplify the demangled symbol of an object wrapped in a variant. */
+inline auto simplify_symbol_variant(Variant const &v) {
+  return boost::apply_visitor(simplify_symbol_visitor{}, v);
+}
+
+/** @brief Simplify the demangled symbol of a container @c value_type. */
+template <typename T> auto simplify_symbol_containee(T const *) {
+  return std::string("");
+}
+
+/** @overload */
+template <typename T> auto simplify_symbol_containee(std::vector<T> const *) {
+  auto const name_val = simplify_symbol(static_cast<T *>(nullptr));
+  return name_val;
+}
+
+/** @overload */
+template <typename K, typename V>
+auto simplify_symbol_containee(std::unordered_map<K, V> const *) {
+  auto const name_key = simplify_symbol(static_cast<K *>(nullptr));
+  auto const name_val = simplify_symbol(static_cast<V *>(nullptr));
+  return name_key + "' or '" + name_val;
+}
+
+struct simplify_symbol_containee_visitor : boost::static_visitor<std::string> {
   template <class T> std::string operator()(const T &) const {
-    auto const symbol_for_variant = Utils::demangle<Variant>();
-    auto const name_for_variant = std::string("ScriptInterface::Variant");
-    auto symbol = Utils::demangle<T>();
-    for (std::string::size_type pos{};
-         (pos = symbol.find(symbol_for_variant, pos)) != symbol.npos;
-         pos += name_for_variant.length()) {
-      symbol.replace(pos, symbol_for_variant.length(), name_for_variant);
-    }
-    return symbol;
+    return simplify_symbol_containee(static_cast<T *>(nullptr));
   }
 };
 
+/**
+ * @brief Simplify the demangled symbol of a container @c value_type wrapped
+ * in a variant.
+ */
+inline auto simplify_symbol_containee_variant(Variant const &v) {
+  return boost::apply_visitor(simplify_symbol_containee_visitor{}, v);
+}
+
+} // namespace demangle
+
 /*
  * Allows
  * T -> T,
@@ -236,6 +261,12 @@ struct get_value_helper<std::unordered_map<int, T>, void> {
     return boost::apply_visitor(GetMapOrEmpty<int, T>{}, v);
   }
 };
+template <typename T>
+struct get_value_helper<std::unordered_map<std::string, T>, void> {
+  std::unordered_map<std::string, T> operator()(Variant const &v) const {
+    return boost::apply_visitor(GetMapOrEmpty<std::string, T>{}, v);
+  }
+};
 
 /** Custom error for a conversion that fails when the value is a nullptr. */
 class bad_get_nullptr : public boost::bad_get {};
@@ -273,24 +304,27 @@ struct get_value_helper<
  * @tparam T     Which type the variant was supposed to convert to
  */
 template <typename T> inline void handle_bad_get(Variant const &v) {
-  auto const object_symbol_name = boost::apply_visitor(type_label_visitor{}, v);
-  auto const element_symbol_name = demangle_container_value_type<T>{}();
-  auto const is_container = !element_symbol_name.empty();
-  auto const what = "Provided argument of type '" + object_symbol_name + "'";
+  auto const container_name = demangle::simplify_symbol_variant(v);
+  auto const containee_name = demangle::simplify_symbol_containee_variant(v);
+  auto const expected_containee_name =
+      demangle::simplify_symbol_containee(static_cast<T *>(nullptr));
+  auto const from_container = !containee_name.empty();
+  auto const to_container = !expected_containee_name.empty();
+  auto const what = "Provided argument of type '" + container_name + "'";
   try {
     throw;
   } catch (bad_get_nullptr const &) {
-    auto const item_error = (is_container) ? " contains a value that" : "";
+    auto const item_error = (to_container) ? " contains a value that" : "";
     throw Exception(what + item_error + " is a null pointer");
   } catch (boost::bad_get const &) {
     auto const non_convertible = std::string(" is not convertible to ");
     auto item_error = std::string("");
-    if (is_container) {
+    if (from_container and to_container) {
       item_error += " because it contains a value that";
-      item_error += non_convertible + "'" + element_symbol_name + "'";
+      item_error += non_convertible + "'" + expected_containee_name + "'";
     }
-    throw Exception(what + non_convertible + "'" + Utils::demangle<T>() + "'" +
-                    item_error);
+    auto const target = demangle::simplify_symbol(static_cast<T *>(nullptr));
+    throw Exception(what + non_convertible + "'" + target + "'" + item_error);
   }
 }
 

src/script_interface/packed_variant.hpp

@@ -55,7 +55,8 @@ using PackedVariant = boost::make_recursive_variant<
     None, bool, int, std::size_t, double, std::string, ObjectId,
     Utils::Vector2d, Utils::Vector3d, Utils::Vector4d, std::vector<int>,
     std::vector<double>, std::vector<boost::recursive_variant_>,
-    std::unordered_map<int, boost::recursive_variant_>>::type;
+    std::unordered_map<int, boost::recursive_variant_>,
+    std::unordered_map<std::string, boost::recursive_variant_>>::type;
 
 using PackedMap = std::vector<std::pair<std::string, PackedVariant>>;
 
@@ -86,8 +87,9 @@ struct PackVisitor : boost::static_visitor<PackedVariant> {
   }
 
   /* For the map, we recurse into each element. */
-  auto operator()(const std::unordered_map<int, Variant> &map) const {
-    std::unordered_map<int, PackedVariant> ret{};
+  template <typename K>
+  auto operator()(const std::unordered_map<K, Variant> &map) const {
+    std::unordered_map<K, PackedVariant> ret{};
 
     for (auto const &it : map) {
       ret.insert({it.first, boost::apply_visitor(*this, it.second)});
@@ -134,8 +136,9 @@ struct UnpackVisitor : boost::static_visitor<Variant> {
   }
 
   /* For the map, we recurse into each element. */
-  auto operator()(const std::unordered_map<int, PackedVariant> &map) const {
-    std::unordered_map<int, Variant> ret{};
+  template <typename K>
+  auto operator()(const std::unordered_map<K, PackedVariant> &map) const {
+    std::unordered_map<K, Variant> ret{};
 
     for (auto const &it : map) {
       ret.insert({it.first, boost::apply_visitor(*this, it.second)});