Skip to content

Commit

Permalink
Improve TypeError exceptions
Browse files Browse the repository at this point in the history
- use pybind11 API instead of Python C API
- provide more informative error messages
  • Loading branch information
rhaschke committed May 23, 2024
1 parent 74b1e5e commit a8896e4
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 19 deletions.
5 changes: 2 additions & 3 deletions core/python/bindings/src/core.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -86,9 +86,8 @@ void setForwardedProperties(Stage& self, const py::object& names) {
for (auto item : names)
s.emplace(item.cast<std::string>());
} catch (const py::cast_error& e) {
// manually translate cast_error to type error
PyErr_SetString(PyExc_TypeError, e.what());
throw py::error_already_set();
// translate cast_error to type_error with an informative message
throw py::type_error("Expecting a string or a list of strings");
}
self.setForwardedProperties(s);
}
Expand Down
31 changes: 15 additions & 16 deletions core/python/bindings/src/properties.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,14 @@ namespace moveit {
namespace python {
namespace {

/** In order to assign new property values in Python, we need to convert the Python object
* to a boost::any instance of the correct type. As the C++ type cannot be inferred from
* the Python type, we can support this assignment only for a few basic types (see fromPython())
* as well as ROS message types. For other types, a generic assignment via
* stage.properties["property"] = value
* is not possible. Instead, use the .property<Type> declaration on the stage to allow for
* direct assignment like this: stage.property = value
**/
class PropertyConverterRegistry
{
struct Entry
Expand Down Expand Up @@ -102,10 +110,8 @@ py::object PropertyConverterRegistry::toPython(const boost::any& value) {

auto it = REGISTRY_SINGLETON.types_.find(value.type());
if (it == REGISTRY_SINGLETON.types_.end()) {
std::string msg("No Python -> C++ conversion for: ");
msg += boost::core::demangle(value.type().name());
PyErr_SetString(PyExc_TypeError, msg.c_str());
throw py::error_already_set();
std::string name = boost::core::demangle(value.type().name());
throw py::type_error("No Python -> C++ conversion for: " + name);
}

return it->second.to_(value);
Expand All @@ -115,12 +121,9 @@ std::string rosMsgName(PyObject* object) {
py::object o = py::reinterpret_borrow<py::object>(object);
try {
return o.attr("_type").cast<std::string>();
} catch (const py::error_already_set&) {
// change error to TypeError
std::string msg = o.attr("__class__").attr("__name__").cast<std::string>();
msg += " is not a ROS message type";
PyErr_SetString(PyExc_TypeError, msg.c_str());
throw py::error_already_set();
} catch (const py::error_already_set& e) {
// object is not a ROS message type, return it's class name instead
return o.attr("__class__").attr("__name__").cast<std::string>();
}
}

Expand All @@ -138,12 +141,8 @@ boost::any PropertyConverterRegistry::fromPython(const py::object& po) {

const std::string& ros_msg_name = rosMsgName(o);
auto it = REGISTRY_SINGLETON.msg_names_.find(ros_msg_name);
if (it == REGISTRY_SINGLETON.msg_names_.end()) {
std::string msg("No Python -> C++ conversion for: ");
msg += ros_msg_name;
PyErr_SetString(PyExc_TypeError, msg.c_str());
throw py::error_already_set();
}
if (it == REGISTRY_SINGLETON.msg_names_.end())
throw py::type_error("No C++ conversion available for (property) type: " + ros_msg_name);

return it->second->second.from_(po);
}
Expand Down
2 changes: 2 additions & 0 deletions core/python/bindings/src/stages.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -323,6 +323,8 @@ void export_stages(pybind11::module& m) {
For an example, see :ref:`How-To-Guides <subsubsec-howto-connect>`.
)")
.property<stages::Connect::MergeMode>("merge_mode", "Defines the merge strategy to use")
.property<double>("max_distance", "maximally accepted distance between end and goal sate")
.def(py::init<const std::string&, const Connect::GroupPlannerVector&>(),
"name"_a = std::string("connect"), "planners"_a);

Expand Down

0 comments on commit a8896e4

Please sign in to comment.