From dbe7506c71734ad19c0f68d971fbfb99109aad2c Mon Sep 17 00:00:00 2001 From: Eric Cousineau Date: Fri, 14 May 2021 09:05:57 -0400 Subject: [PATCH] fixup! make test pass, simplify overloads --- bindings/pydrake/symbolic_py.cc | 7 +-- bindings/pydrake/symbolic_types_pybind.h | 12 ++--- bindings/pydrake/test/math_overloads_test.py | 47 +++++++++++++------- 3 files changed, 35 insertions(+), 31 deletions(-) diff --git a/bindings/pydrake/symbolic_py.cc b/bindings/pydrake/symbolic_py.cc index d7ff88f21111..2e87c9cd23b4 100644 --- a/bindings/pydrake/symbolic_py.cc +++ b/bindings/pydrake/symbolic_py.cc @@ -408,14 +408,11 @@ PYBIND11_MODULE(symbolic, m) { .def("Differentiate", &Expression::Differentiate, doc.Expression.Differentiate.doc) .def("Jacobian", &Expression::Jacobian, doc.Expression.Jacobian.doc); + // TODO(eric.cousineau): Clean this overload stuff up (#15041). pydrake::internal::BindSymbolicMathOverloads(&expr_cls); + pydrake::internal::BindSymbolicMathOverloads(&m); DefCopyAndDeepCopy(&expr_cls); - // TODO(eric.cousineau): These should actually exist on the class, and should - // be should be consolidated with the above repeated definitions. This would - // yield the same parity with AutoDiff. - pydrake::internal::BindSymbolicMathModuleOverloads(m); - m.def("if_then_else", &symbolic::if_then_else); m.def( diff --git a/bindings/pydrake/symbolic_types_pybind.h b/bindings/pydrake/symbolic_types_pybind.h index 4a879530ad0d..b366c28be56e 100644 --- a/bindings/pydrake/symbolic_types_pybind.h +++ b/bindings/pydrake/symbolic_types_pybind.h @@ -64,15 +64,9 @@ void BindSymbolicMathOverloads(PyObject* obj) { .def("ceil", &symbolic::ceil, doc.ceil.doc) .def("__ceil__", &symbolic::ceil, doc.ceil.doc) .def("floor", &symbolic::floor, doc.floor.doc) - .def("__floor__", &symbolic::floor, doc.floor.doc); -} - -inline void BindSymbolicMathModuleOverloads(py::module m) { - using symbolic::Expression; - BindSymbolicMathOverloads(&m); - m // BR - // TODO(eric.cousineau): This is not a NumPy-overridable method using - // dtype=object. Deprecate and move solely into `pydrake.math`. + .def("__floor__", &symbolic::floor, doc.floor.doc) + // TODO(eric.cousineau): This is not a NumPy-overridable method using + // dtype=object. Deprecate and move solely into `pydrake.math`. .def( "inv", [](const MatrixX& X) -> MatrixX { diff --git a/bindings/pydrake/test/math_overloads_test.py b/bindings/pydrake/test/math_overloads_test.py index 8e859ac6005c..da7cd9bb6cf3 100644 --- a/bindings/pydrake/test/math_overloads_test.py +++ b/bindings/pydrake/test/math_overloads_test.py @@ -103,7 +103,7 @@ def supports(self, func): supported = backwards_compat if func.__name__ in backwards_compat: # Check backwards compatibility. - assert hasattr(self.m, func.__name__) + assert hasattr(self.m, func.__name__), self.m.__name__ return func.__name__ in supported def to_float(self, y_T): @@ -158,22 +158,35 @@ def check_eval(functions, nargs): args_T = list(map(overload.to_type, args_float)) # Check each supported function. for f_drake, f_builtin in functions: - if not overload.supports(f_drake): - continue - debug_print( - "- Functions: ", qualname(f_drake), qualname(f_builtin)) - y_builtin = f_builtin(*args_float) - y_float = f_drake(*args_float) - debug_print(" - - Float Eval:", repr(y_builtin), repr(y_float)) - self.assertEqual(y_float, y_builtin) - self.assertIsInstance(y_float, float) - # Test method current overload, and ensure value is accurate. - y_T = f_drake(*args_T) - y_T_float = overload.to_float(y_T) - debug_print(" - - Overload Eval:", repr(y_T), repr(y_T_float)) - self.assertIsInstance(y_T, overload.T) - # - Ensure the translated value is accurate. - self.assertEqual(y_T_float, y_float) + with self.subTest(function=f_drake.__name__, nargs=nargs): + if not overload.supports(f_drake): + continue + debug_print( + "- Functions: ", + qualname(f_drake), + qualname(f_builtin), + ) + y_builtin = f_builtin(*args_float) + y_float = f_drake(*args_float) + debug_print( + " - - Float Eval:", + repr(y_builtin), + repr(y_float), + ) + self.assertEqual(y_float, y_builtin) + self.assertIsInstance(y_float, float) + # Test method current overload, and ensure value is + # accurate. + y_T = f_drake(*args_T) + y_T_float = overload.to_float(y_T) + debug_print( + " - - Overload Eval:", + repr(y_T), + repr(y_T_float), + ) + self.assertIsInstance(y_T, overload.T) + # - Ensure the translated value is accurate. + self.assertEqual(y_T_float, y_float) debug_print("\n\nOverload: ", qualname(type(overload))) float_overload = FloatOverloads()