From e9e17746c8015fb567be72e90edb625be7eb3321 Mon Sep 17 00:00:00 2001 From: Jason Rhinelander Date: Sat, 8 Apr 2017 19:26:42 -0400 Subject: [PATCH] Fix Eigen argument doc strings Many of the Eigen type casters' name() methods weren't wrapping the type description in a `type_descr` object, which thus wasn't adding the "{...}" annotation used to identify an argument which broke the help output by skipping eigen arguments. The test code I had added even had some (unnoticed) broken output (with the "arg0: " showing up in the return value). This commit also adds test code to ensure that named eigen arguments actually work properly, despite the invalid help output. (The added tests pass without the rest of this commit). --- include/pybind11/eigen.h | 31 ++++++++++++++++--------------- tests/test_eigen.cpp | 7 +++++++ tests/test_eigen.py | 29 ++++++++++++++++++++++++++--- 3 files changed, 49 insertions(+), 18 deletions(-) diff --git a/include/pybind11/eigen.h b/include/pybind11/eigen.h index 6abe8c48f..69194a2c4 100644 --- a/include/pybind11/eigen.h +++ b/include/pybind11/eigen.h @@ -179,20 +179,21 @@ template struct EigenProps { constexpr bool show_c_contiguous = show_order && requires_row_major; constexpr bool show_f_contiguous = !show_c_contiguous && show_order && requires_col_major; - return _("numpy.ndarray[") + npy_format_descriptor::name() + - _("[") + _(_<(size_t) rows>(), _("m")) + - _(", ") + _(_<(size_t) cols>(), _("n")) + - _("]") + - // For a reference type (e.g. Ref) we have other constraints that might need to be - // satisfied: writeable=True (for a mutable reference), and, depending on the map's stride - // options, possibly f_contiguous or c_contiguous. We include them in the descriptor output - // to provide some hint as to why a TypeError is occurring (otherwise it can be confusing to - // see that a function accepts a 'numpy.ndarray[float64[3,2]]' and an error message that you - // *gave* a numpy.ndarray of the right type and dimensions. - _(", flags.writeable", "") + - _(", flags.c_contiguous", "") + - _(", flags.f_contiguous", "") + - _("]"); + return type_descr(_("numpy.ndarray[") + npy_format_descriptor::name() + + _("[") + _(_<(size_t) rows>(), _("m")) + + _(", ") + _(_<(size_t) cols>(), _("n")) + + _("]") + + // For a reference type (e.g. Ref) we have other constraints that might need to be + // satisfied: writeable=True (for a mutable reference), and, depending on the map's stride + // options, possibly f_contiguous or c_contiguous. We include them in the descriptor output + // to provide some hint as to why a TypeError is occurring (otherwise it can be confusing to + // see that a function accepts a 'numpy.ndarray[float64[3,2]]' and an error message that you + // *gave* a numpy.ndarray of the right type and dimensions. + _(", flags.writeable", "") + + _(", flags.c_contiguous", "") + + _(", flags.f_contiguous", "") + + _("]") + ); } }; @@ -318,7 +319,7 @@ public: return cast_impl(src, policy, parent); } - static PYBIND11_DESCR name() { return type_descr(props::descriptor()); } + static PYBIND11_DESCR name() { return props::descriptor(); } operator Type*() { return &value; } operator Type&() { return value; } diff --git a/tests/test_eigen.cpp b/tests/test_eigen.cpp index f2ec8fd2e..52358ab6f 100644 --- a/tests/test_eigen.cpp +++ b/tests/test_eigen.cpp @@ -287,6 +287,13 @@ test_initializer eigen([](py::module &m) { m.def("iss738_f1", &adjust_matrix &>, py::arg().noconvert()); m.def("iss738_f2", &adjust_matrix> &>, py::arg().noconvert()); + // Make sure named arguments are working properly: + m.def("matrix_multiply", [](const py::EigenDRef A, const py::EigenDRef B) + -> Eigen::MatrixXd { + if (A.cols() != B.rows()) throw std::domain_error("Nonconformable matrices!"); + return A * B; + }, py::arg("A"), py::arg("B")); + py::class_(m, "CustomOperatorNew") .def(py::init<>()) .def_readonly("a", &CustomOperatorNew::a) diff --git a/tests/test_eigen.py b/tests/test_eigen.py index df08edf3d..ffa678bed 100644 --- a/tests/test_eigen.py +++ b/tests/test_eigen.py @@ -75,15 +75,15 @@ def test_mutator_descriptors(): fixed_mutator_a(zc) with pytest.raises(TypeError) as excinfo: fixed_mutator_r(zc) - assert ('(numpy.ndarray[float32[5, 6], flags.writeable, flags.c_contiguous]) -> arg0: None' + assert ('(arg0: numpy.ndarray[float32[5, 6], flags.writeable, flags.c_contiguous]) -> None' in str(excinfo.value)) with pytest.raises(TypeError) as excinfo: fixed_mutator_c(zr) - assert ('(numpy.ndarray[float32[5, 6], flags.writeable, flags.f_contiguous]) -> arg0: None' + assert ('(arg0: numpy.ndarray[float32[5, 6], flags.writeable, flags.f_contiguous]) -> None' in str(excinfo.value)) with pytest.raises(TypeError) as excinfo: fixed_mutator_a(np.array([[1, 2], [3, 4]], dtype='float32')) - assert ('(numpy.ndarray[float32[5, 6], flags.writeable]) -> arg0: None' + assert ('(arg0: numpy.ndarray[float32[5, 6], flags.writeable]) -> None' in str(excinfo.value)) zr.flags.writeable = False with pytest.raises(TypeError): @@ -582,6 +582,29 @@ def test_dense_signature(doc): """ +def test_named_arguments(): + from pybind11_tests import matrix_multiply + + a = np.array([[1.0, 2], [3, 4], [5, 6]]) + b = np.ones((2, 1)) + + assert np.all(matrix_multiply(a, b) == np.array([[3.], [7], [11]])) + assert np.all(matrix_multiply(A=a, B=b) == np.array([[3.], [7], [11]])) + assert np.all(matrix_multiply(B=b, A=a) == np.array([[3.], [7], [11]])) + + with pytest.raises(ValueError) as excinfo: + matrix_multiply(b, a) + assert str(excinfo.value) == 'Nonconformable matrices!' + + with pytest.raises(ValueError) as excinfo: + matrix_multiply(A=b, B=a) + assert str(excinfo.value) == 'Nonconformable matrices!' + + with pytest.raises(ValueError) as excinfo: + matrix_multiply(B=a, A=b) + assert str(excinfo.value) == 'Nonconformable matrices!' + + @pytest.requires_eigen_and_scipy def test_sparse(): from pybind11_tests import sparse_r, sparse_c, sparse_copy_r, sparse_copy_c