From 6a81dbbb1e07ff0f37f362ce426cd54910536f4f Mon Sep 17 00:00:00 2001 From: Jason Rhinelander Date: Thu, 21 Sep 2017 19:09:39 -0300 Subject: [PATCH] Fix 2D Nx1/1xN inputs to eigen dense vector args This fixes a bug introduced in b68959e822a619d1ad140f52c7197a29f4e6a06a when passing in a two-dimensional, but conformable, array as the value for a compile-time Eigen vector (such as VectorXd or RowVectorXd). The commit switched to using numpy to copy into the eigen data, but this broke the described case because numpy refuses to broadcast a (N,1) into a (N). This commit fixes it by squeezing the input array whenever the output array is 1-dimensional, which will let the problematic case through. (This shouldn't squeeze inappropriately as dimension compatibility is already checked for conformability before getting to the copy code). --- include/pybind11/eigen.h | 1 + tests/test_eigen.cpp | 7 +++++++ tests/test_eigen.py | 15 +++++++++++++++ 3 files changed, 23 insertions(+) diff --git a/include/pybind11/eigen.h b/include/pybind11/eigen.h index 8190246b..2a234cef 100644 --- a/include/pybind11/eigen.h +++ b/include/pybind11/eigen.h @@ -270,6 +270,7 @@ struct type_caster::value>> { value = Type(fits.rows, fits.cols); auto ref = reinterpret_steal(eigen_ref_array(value)); if (dims == 1) ref = ref.squeeze(); + else if (ref.ndim() == 1) buf = buf.squeeze(); int result = detail::npy_api::get().PyArray_CopyInto_(ref.ptr(), buf.ptr()); diff --git a/tests/test_eigen.cpp b/tests/test_eigen.cpp index 17b156ce..20be0aa1 100644 --- a/tests/test_eigen.cpp +++ b/tests/test_eigen.cpp @@ -288,6 +288,13 @@ TEST_SUBMODULE(eigen, m) { m.def("iss738_f1", &adjust_matrix &>, py::arg().noconvert()); m.def("iss738_f2", &adjust_matrix> &>, py::arg().noconvert()); + // test_issue1105 + // Issue #1105: when converting from a numpy two-dimensional (Nx1) or (1xN) value into a dense + // eigen Vector or RowVector, the argument would fail to load because the numpy copy would fail: + // numpy won't broadcast a Nx1 into a 1-dimensional vector. + m.def("iss1105_col", [](Eigen::VectorXd) { return true; }); + m.def("iss1105_row", [](Eigen::RowVectorXd) { return true; }); + // test_named_arguments // Make sure named arguments are working properly: m.def("matrix_multiply", [](const py::EigenDRef A, const py::EigenDRef B) diff --git a/tests/test_eigen.py b/tests/test_eigen.py index 4ac8cbf5..1f263db4 100644 --- a/tests/test_eigen.py +++ b/tests/test_eigen.py @@ -672,6 +672,21 @@ def test_issue738(): assert np.all(m.iss738_f2(np.array([[1.], [2], [3]])) == np.array([[1.], [12], [23]])) +def test_issue1105(): + """Issue 1105: 1xN or Nx1 input arrays weren't accepted for eigen + compile-time row vectors or column vector""" + assert m.iss1105_row(np.ones((1, 7))) + assert m.iss1105_col(np.ones((7, 1))) + + # These should still fail (incompatible dimensions): + with pytest.raises(TypeError) as excinfo: + m.iss1105_row(np.ones((7, 1))) + assert "incompatible function arguments" in str(excinfo) + with pytest.raises(TypeError) as excinfo: + m.iss1105_col(np.ones((1, 7))) + assert "incompatible function arguments" in str(excinfo) + + def test_custom_operator_new(): """Using Eigen types as member variables requires a class-specific operator new with proper alignment"""