Skip to content

Commit 9b976cc

Browse files
committed
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).
1 parent 501135f commit 9b976cc

File tree

3 files changed

+46
-15
lines changed

3 files changed

+46
-15
lines changed

include/pybind11/eigen.h

Lines changed: 16 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -179,20 +179,21 @@ template <typename Type_> struct EigenProps {
179179
constexpr bool show_c_contiguous = show_order && requires_row_major;
180180
constexpr bool show_f_contiguous = !show_c_contiguous && show_order && requires_col_major;
181181

182-
return _("numpy.ndarray[") + npy_format_descriptor<Scalar>::name() +
183-
_("[") + _<fixed_rows>(_<(size_t) rows>(), _("m")) +
184-
_(", ") + _<fixed_cols>(_<(size_t) cols>(), _("n")) +
185-
_("]") +
186-
// For a reference type (e.g. Ref<MatrixXd>) we have other constraints that might need to be
187-
// satisfied: writeable=True (for a mutable reference), and, depending on the map's stride
188-
// options, possibly f_contiguous or c_contiguous. We include them in the descriptor output
189-
// to provide some hint as to why a TypeError is occurring (otherwise it can be confusing to
190-
// see that a function accepts a 'numpy.ndarray[float64[3,2]]' and an error message that you
191-
// *gave* a numpy.ndarray of the right type and dimensions.
192-
_<show_writeable>(", flags.writeable", "") +
193-
_<show_c_contiguous>(", flags.c_contiguous", "") +
194-
_<show_f_contiguous>(", flags.f_contiguous", "") +
195-
_("]");
182+
return type_descr(_("numpy.ndarray[") + npy_format_descriptor<Scalar>::name() +
183+
_("[") + _<fixed_rows>(_<(size_t) rows>(), _("m")) +
184+
_(", ") + _<fixed_cols>(_<(size_t) cols>(), _("n")) +
185+
_("]") +
186+
// For a reference type (e.g. Ref<MatrixXd>) we have other constraints that might need to be
187+
// satisfied: writeable=True (for a mutable reference), and, depending on the map's stride
188+
// options, possibly f_contiguous or c_contiguous. We include them in the descriptor output
189+
// to provide some hint as to why a TypeError is occurring (otherwise it can be confusing to
190+
// see that a function accepts a 'numpy.ndarray[float64[3,2]]' and an error message that you
191+
// *gave* a numpy.ndarray of the right type and dimensions.
192+
_<show_writeable>(", flags.writeable", "") +
193+
_<show_c_contiguous>(", flags.c_contiguous", "") +
194+
_<show_f_contiguous>(", flags.f_contiguous", "") +
195+
_("]")
196+
);
196197
}
197198
};
198199

@@ -318,7 +319,7 @@ struct type_caster<Type, enable_if_t<is_eigen_dense_plain<Type>::value>> {
318319
return cast_impl(src, policy, parent);
319320
}
320321

321-
static PYBIND11_DESCR name() { return type_descr(props::descriptor()); }
322+
static PYBIND11_DESCR name() { return props::descriptor(); }
322323

323324
operator Type*() { return &value; }
324325
operator Type&() { return value; }

tests/test_eigen.cpp

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -287,6 +287,13 @@ test_initializer eigen([](py::module &m) {
287287
m.def("iss738_f1", &adjust_matrix<const Eigen::Ref<const Eigen::MatrixXd> &>, py::arg().noconvert());
288288
m.def("iss738_f2", &adjust_matrix<const Eigen::Ref<const Eigen::Matrix<double, -1, -1, Eigen::RowMajor>> &>, py::arg().noconvert());
289289

290+
// Make sure named arguments are working properly:
291+
m.def("matrix_multiply", [](const py::EigenDRef<const Eigen::MatrixXd> A, const py::EigenDRef<const Eigen::MatrixXd> B)
292+
-> Eigen::MatrixXd {
293+
if (A.cols() != B.rows()) throw std::domain_error("Nonconformable matrices!");
294+
return A * B;
295+
}, py::arg("A"), py::arg("B"));
296+
290297
py::class_<CustomOperatorNew>(m, "CustomOperatorNew")
291298
.def(py::init<>())
292299
.def_readonly("a", &CustomOperatorNew::a)

tests/test_eigen.py

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -582,6 +582,29 @@ def test_dense_signature(doc):
582582
"""
583583

584584

585+
def test_named_arguments():
586+
from pybind11_tests import matrix_multiply
587+
588+
myA = np.array([[1.0, 2], [3, 4], [5, 6]])
589+
myB = np.ones((2, 1))
590+
591+
assert np.all(matrix_multiply(myA, myB) == np.array([[3.], [7], [11]]))
592+
assert np.all(matrix_multiply(A=myA, B=myB) == np.array([[3.], [7], [11]]))
593+
assert np.all(matrix_multiply(B=myB, A=myA) == np.array([[3.], [7], [11]]))
594+
595+
with pytest.raises(ValueError) as excinfo:
596+
matrix_multiply(myB, myA)
597+
assert str(excinfo.value) == 'Nonconformable matrices!'
598+
599+
with pytest.raises(ValueError) as excinfo:
600+
matrix_multiply(A=myB, B=myA)
601+
assert str(excinfo.value) == 'Nonconformable matrices!'
602+
603+
with pytest.raises(ValueError) as excinfo:
604+
matrix_multiply(B=myA, A=myB)
605+
assert str(excinfo.value) == 'Nonconformable matrices!'
606+
607+
585608
@pytest.requires_eigen_and_scipy
586609
def test_sparse():
587610
from pybind11_tests import sparse_r, sparse_c, sparse_copy_r, sparse_copy_c

0 commit comments

Comments
 (0)