Merge pull request #13194 from EricCousineau-TRI/feature-py-nice-erro…

…r-scalar-convert

pydrake: Override NiceTypeName::Get(ptr) when possible

bindings/pydrake/common/BUILD.bazel

@@ -47,6 +47,7 @@ drake_pybind_library(
         "//bindings/pydrake:documentation_pybind",
         "//bindings/pydrake/common:deprecation_pybind",
         "//bindings/pydrake/common:value_pybind",
+        "//common:nice_type_name_override_header",
     ],
     cc_so_name = "_module_py",
     cc_srcs = [

bindings/pydrake/common/module_py.cc

@@ -9,21 +9,83 @@
 #include "drake/common/drake_assertion_error.h"
 #include "drake/common/drake_path.h"
 #include "drake/common/find_resource.h"
+#include "drake/common/nice_type_name.h"
+#include "drake/common/nice_type_name_override.h"
 #include "drake/common/random.h"
 #include "drake/common/temp_directory.h"
 #include "drake/common/text_logging.h"
 
 namespace drake {
 namespace pydrake {
 
+using drake::internal::type_erased_ptr;
+
 // This function is defined in drake/common/drake_assert_and_throw.cc.
 extern "C" void drake_set_assertion_failure_to_throw_exception();
 
 namespace {
+
 void trigger_an_assertion_failure() {
   DRAKE_DEMAND(false);
 }
-}  // namespace
+
+// Resolves to a Python handle given a type erased pointer. If the instance or
+// lowest-level RTTI type are unregistered, returns an empty handle.
+py::handle ResolvePyObject(const type_erased_ptr& ptr) {
+  auto py_type_info = py::detail::get_type_info(ptr.info);
+  return py::detail::get_object_handle(ptr.raw, py_type_info);
+}
+
+// Gets a class's fully-qualified name.
+std::string GetPyClassName(py::handle obj) {
+  DRAKE_DEMAND(!!obj);
+  py::handle type = py::module::import("builtins").attr("type");
+  py::handle cls = type(obj);
+  return py::str("{}.{}").format(
+      cls.attr("__module__"), cls.attr("__qualname__"));
+}
+
+// Override for SetNiceTypeNamePtrOverride, to ensure that instances that are
+// registered (along with their types) can use their Python class's name.
+std::string PyNiceTypeNamePtrOverride(const type_erased_ptr& ptr) {
+  DRAKE_DEMAND(ptr.raw != nullptr);
+  const std::string cc_name = NiceTypeName::Get(ptr.info);
+  if (cc_name.find("pydrake::") != std::string::npos) {
+    py::handle obj = ResolvePyObject(ptr);
+    if (obj) {
+      return GetPyClassName(obj);
+    }
+  }
+  return cc_name;
+}
+
+namespace testing {
+// Registered type. Also a base class for UnregisteredDerivedType.
+class RegisteredType {
+ public:
+  virtual ~RegisteredType() {}
+};
+// Completely unregistered type.
+class UnregisteredType {};
+// Unregistered type, but with a registered base.
+class UnregisteredDerivedType : public RegisteredType {};
+
+void def_testing(py::module m) {
+  py::class_<RegisteredType>(m, "RegisteredType").def(py::init());
+  // See comments in `module_test.py`.
+  m.def("get_nice_type_name_cc_registered_instance",
+      [](const RegisteredType& obj) { return NiceTypeName::Get(obj); });
+  m.def("get_nice_type_name_cc_unregistered_instance",
+      []() { return NiceTypeName::Get(RegisteredType()); });
+  m.def("get_nice_type_name_cc_typeid",
+      [](const RegisteredType& obj) { return NiceTypeName::Get(typeid(obj)); });
+  m.def("get_nice_type_name_cc_unregistered_type",
+      []() { return NiceTypeName::Get(UnregisteredType()); });
+  m.def("make_cc_unregistered_derived_type", []() {
+    return std::unique_ptr<RegisteredType>(new UnregisteredDerivedType());
+  });
+}
+}  // namespace testing
 
 PYBIND11_MODULE(_module_py, m) {
   PYDRAKE_PREVENT_PYTHON3_MODULE_REIMPORT(m);
@@ -112,7 +174,15 @@ PYBIND11_MODULE(_module_py, m) {
       "Trigger a Drake C++ assertion failure");
 
   m.attr("kDrakeAssertIsArmed") = kDrakeAssertIsArmed;
+
+  // Make nice_type_name use Python type info when available.
+  drake::internal::SetNiceTypeNamePtrOverride(PyNiceTypeNamePtrOverride);
+
+  // Define testing.
+  py::module m_testing = m.def_submodule("_testing");
+  testing::def_testing(m_testing);
 }
 
+}  // namespace
 }  // namespace pydrake
 }  // namespace drake

bindings/pydrake/common/test/module_test.py

@@ -2,6 +2,7 @@
 import unittest
 
 import pydrake.common as mut
+import pydrake.common._module_py._testing as mut_testing
 
 
 class TestCommon(unittest.TestCase):
@@ -56,3 +57,43 @@ def test_random_generator(self):
 
     def test_assert_is_armed(self):
         self.assertIsInstance(mut.kDrakeAssertIsArmed, bool)
+
+    def test_nice_type_name(self):
+        """Tests behavior of ``PyNiceTypeNamePtrOverride`` in module_py.cc."""
+        obj = mut_testing.RegisteredType()
+        registered_type_py_name = f"{mut_testing.__name__}.RegisteredType"
+        registered_type_cc_name = (
+            "drake::pydrake::(anonymous)::testing::RegisteredType")
+        unregistered_type_cc_name = (
+            "drake::pydrake::(anonymous)::testing::UnregisteredType")
+        unregistered_derived_type_cc_name = (
+            "drake::pydrake::(anonymous)::testing::UnregisteredDerivedType")
+        # Type and instance are registered with Python, so it should return the
+        # Python type name.
+        self.assertEqual(
+            mut_testing.get_nice_type_name_cc_registered_instance(obj),
+            registered_type_py_name)
+        # Type is known, but instance is unregistered, so it should return the
+        # C++ type name.
+        self.assertEqual(
+            mut_testing.get_nice_type_name_cc_unregistered_instance(),
+            registered_type_cc_name)
+        # Uses raw typeid for a registered type, so it should return the C++
+        # type name.
+        self.assertEqual(
+            mut_testing.get_nice_type_name_cc_typeid(obj),
+            registered_type_cc_name)
+        # Type and instance are unregistered, so it should return the C++ type
+        # name.
+        self.assertEqual(
+            mut_testing.get_nice_type_name_cc_unregistered_type(),
+            unregistered_type_cc_name)
+        # Type is unregistered but derived from a registered base type (to
+        # mimic Value<> / AbstractValue), and instance is registered. Should
+        # return C++ type name.
+        base_only_instance = mut_testing.make_cc_unregistered_derived_type()
+        self.assertIs(type(base_only_instance), mut_testing.RegisteredType)
+        self.assertEqual(
+            mut_testing.get_nice_type_name_cc_registered_instance(
+                base_only_instance),
+            unregistered_derived_type_cc_name)

bindings/pydrake/common/test/value_test.py

@@ -4,7 +4,7 @@
 from pydrake.common.value import AbstractValue, Value
 
 from pydrake.common.test.value_test_util import (
-    make_unknown_abstract_value,
+    make_abstract_value_cc_type_unregistered,
     MoveOnlyType,
 )
 
@@ -74,15 +74,15 @@ def test_abstract_value_make(self):
         value = AbstractValue.Make({"x": 10})
         self.assertIsInstance(value, Value[object])
 
-    def test_abstract_value_unknown(self):
-        value = make_unknown_abstract_value()
+    def test_abstract_value_cc_type_unregistered(self):
+        value = make_abstract_value_cc_type_unregistered()
         self.assertIsInstance(value, AbstractValue)
         with self.assertRaises(RuntimeError) as cm:
             value.get_value()
         self.assertTrue(all(
             s in str(cm.exception) for s in [
                 "AbstractValue",
-                "UnknownType",
+                "UnregisteredType",
                 "get_value",
                 "AddValueInstantiation",
             ]), cm.exception)

bindings/pydrake/common/test/value_test_util_py.cc

@@ -24,7 +24,10 @@ class MoveOnlyType {
   int x_{};
 };
 
-struct UnknownType {};
+// This type is explicitly not registered with pybind11.
+struct UnregisteredType {
+  int junk{};
+};
 
 }  // namespace
 
@@ -38,8 +41,8 @@ PYBIND11_MODULE(value_test_util, m) {
   // Define `Value` instantiation.
   AddValueInstantiation<MoveOnlyType>(m);
 
-  m.def("make_unknown_abstract_value",
-      []() { return AbstractValue::Make(UnknownType{}); });
+  m.def("make_abstract_value_cc_type_unregistered",
+      []() { return AbstractValue::Make(UnregisteredType{}); });
 }
 
 }  // namespace pydrake

bindings/pydrake/systems/framework_py_systems.cc

@@ -274,6 +274,8 @@ struct Impl {
     auto system_cls = DefineTemplateClassWithDefault<System<T>, PySystem>(
         m, "System", GetPyParam<T>(), doc.SystemBase.doc);
     system_cls  // BR
+        .def("GetSystemType", &System<T>::GetSystemType,
+            doc.SystemBase.GetSystemType.doc)
         .def("get_name", &System<T>::get_name, doc.SystemBase.get_name.doc)
         .def("set_name", &System<T>::set_name, doc.SystemBase.set_name.doc)
         // Topology.

bindings/pydrake/systems/test/custom_test.py

@@ -133,13 +133,15 @@ def _fix_adder_inputs(self, system, context):
 
     def test_diagram_adder(self):
         system = CustomDiagram(2, 3)
+        self.assertEqual(system.GetSystemType(), f"{__name__}.CustomDiagram")
         self.assertEqual(system.num_input_ports(), 2)
         self.assertEqual(system.get_input_port(0).size(), 3)
         self.assertEqual(system.num_output_ports(), 1)
         self.assertEqual(system.get_output_port(0).size(), 3)
 
     def test_adder_execution(self):
         system = self._create_adder_system()
+        self.assertEqual(system.GetSystemType(), f"{__name__}.CustomAdder")
         context = system.CreateDefaultContext()
         self.assertEqual(context.num_output_ports(), 1)
         self._fix_adder_inputs(system, context)
@@ -382,6 +384,8 @@ def test_vector_system_overrides(self):
         dt = 0.5
         for is_discrete in [False, True]:
             system = CustomVectorSystem(is_discrete)
+            self.assertEqual(
+                system.GetSystemType(), f"{__name__}.CustomVectorSystem")
             context = system.CreateDefaultContext()
 
             u = np.array([1.])

bindings/pydrake/systems/test/general_test.py

@@ -94,6 +94,9 @@ def _compare_system_instances(self, lhs, rhs):
     def test_system_base_api(self):
         # Test a system with a different number of inputs from outputs.
         system = Adder(3, 10)
+        self.assertEqual(
+            system.GetSystemType(),
+            "drake::systems::Adder<double>")
         self.assertEqual(system.num_input_ports(), 3)
         self.assertEqual(system.num_output_ports(), 1)
         u1 = system.GetInputPort("u1")

bindings/pydrake/systems/test/scalar_conversion_test.py

@@ -64,6 +64,12 @@ def test_example_system(self):
         param_list = [(T,) for T in SystemScalarConverter.SupportedScalars]
         self.assertListEqual(Example_.param_list, param_list)
 
+        for T in SystemScalarConverter.SupportedScalars:
+            system_T = Example_[T](0)
+            self.assertEqual(
+                system_T.GetSystemType(),
+                f"pydrake.systems.scalar_conversion.Example_[{T.__name__}]")
+
         # Test private properties (do NOT use these in your code!).
         self.assertTupleEqual(
             tuple(Example_._T_list), SystemScalarConverter.SupportedScalars)

common/BUILD.bazel

@@ -397,12 +397,30 @@ drake_cc_library(
     ],
 )
 
+# N.B. This library does not have all of its dependencies declared. Instead,
+# it defines only the headers such that it can be used by `pydrake` without
+# installing the file. (If we just used `install_hdrs_exclude`, the header
+# would not make it into `//:drake_shared_library`.)
+drake_cc_library(
+    name = "nice_type_name_override_header",
+    hdrs = ["nice_type_name_override.h"],
+    install_hdrs_exclude = ["nice_type_name_override.h"],
+    tags = ["exclude_from_package"],
+    visibility = ["//bindings/pydrake/common:__pkg__"],
+)
+
 drake_cc_library(
     name = "nice_type_name",
-    srcs = ["nice_type_name.cc"],
-    hdrs = ["nice_type_name.h"],
+    srcs = [
+        "nice_type_name.cc",
+        "nice_type_name_override.cc",
+    ],
+    hdrs = [
+        "nice_type_name.h",
+    ],
     deps = [
         ":essential",
+        ":nice_type_name_override_header",
     ],
 )
 

common/nice_type_name.cc

@@ -14,6 +14,7 @@ copy of the License at http://www.apache.org/licenses/LICENSE-2.0. */
 #include <string>
 
 #include "drake/common/never_destroyed.h"
+#include "drake/common/nice_type_name_override.h"
 
 using std::string;
 
@@ -31,7 +32,7 @@ namespace drake {
 // On gcc and clang typeid(T).name() returns an indecipherable mangled string
 // that requires processing to become human readable. Microsoft returns a
 // reasonable name directly.
-string drake::NiceTypeName::Demangle(const char* typeid_name) {
+string NiceTypeName::Demangle(const char* typeid_name) {
 #if defined(__GNUG__)
   int status = -100;  // just in case it doesn't get set
   char* ret = abi::__cxa_demangle(typeid_name, NULL, NULL, &status);
@@ -48,7 +49,7 @@ string drake::NiceTypeName::Demangle(const char* typeid_name) {
 // Given a demangled string, attempt to canonicalize it for platform
 // indpendence. We'll remove Microsoft's "class ", "struct ", etc.
 // designations, and get rid of all unnecessary spaces.
-string drake::NiceTypeName::Canonicalize(const string& demangled) {
+string NiceTypeName::Canonicalize(const string& demangled) {
   using SPair = std::pair<std::regex, string>;
   using SPairList = std::initializer_list<SPair>;
   // These are applied in this order.
@@ -98,7 +99,7 @@ string drake::NiceTypeName::Canonicalize(const string& demangled) {
   return canonical;
 }
 
-string drake::NiceTypeName::RemoveNamespaces(const string& canonical) {
+string NiceTypeName::RemoveNamespaces(const string& canonical) {
   // Removes everything up to and including the last "::" that isn't part of a
   // template argument. If the string ends with "::", returns the original
   // string unprocessed. We are depending on the fact that namespaces can't be
@@ -112,5 +113,15 @@ string drake::NiceTypeName::RemoveNamespaces(const string& canonical) {
   return no_namespace.empty() ? canonical : no_namespace;
 }
 
-}  // namespace drake
+std::string NiceTypeName::GetWithPossibleOverride(
+      const void* ptr, const std::type_info& info) {
+  internal::NiceTypeNamePtrOverride ptr_override =
+      internal::GetNiceTypeNamePtrOverride();
+  if (ptr_override) {
+    return ptr_override(internal::type_erased_ptr{ptr, info});
+  } else {
+    return Get(info);
+  }
+}
 
+}  // namespace drake

common/nice_type_name.h

@@ -64,7 +64,7 @@ class NiceTypeName {
   differ. */
   template <typename T>
   static std::string Get(const T& thing) {
-    return Get(typeid(thing));
+    return GetWithPossibleOverride(&thing, typeid(thing));
   }
 
   /** Returns the nicely demangled and canonicalized type name of `info`. This
@@ -101,6 +101,9 @@ class NiceTypeName {
  private:
   // No instances of this class should be created.
   NiceTypeName() = delete;
+
+  static std::string GetWithPossibleOverride(
+      const void* ptr, const std::type_info& info);
 };
 
 }  // namespace drake