diff --git a/include/pybind11/eigen.h b/include/pybind11/eigen.h index 71fc6f1dc..e9c44e565 100644 --- a/include/pybind11/eigen.h +++ b/include/pybind11/eigen.h @@ -79,11 +79,17 @@ template struct EigenConformable { EigenRowMajor ? cstride : rstride /* inner stride */) {} // Vector type: - EigenConformable(EigenIndex r, EigenIndex c, EigenIndex stride) : EigenConformable(r, c, r == 1 ? c*stride : stride, c == 1 ? r : r*stride) {} + EigenConformable(EigenIndex r, EigenIndex c, EigenIndex stride) + : EigenConformable(r, c, r == 1 ? c*stride : stride, c == 1 ? r : r*stride) {} + template bool stride_compatible() const { + // To have compatible strides, we need (on both dimensions) one of fully dynamic strides, + // matching strides, or a dimension size of 1 (in which case the stride value is irrelevant) return - (props::inner_stride == Eigen::Dynamic || props::inner_stride == stride.inner()) && - (props::outer_stride == Eigen::Dynamic || props::outer_stride == stride.outer()); + (props::inner_stride == Eigen::Dynamic || props::inner_stride == stride.inner() || + (EigenRowMajor ? cols : rows) == 1) && + (props::outer_stride == Eigen::Dynamic || props::outer_stride == stride.outer() || + (EigenRowMajor ? rows : cols) == 1); } operator bool() const { return conformable; } }; diff --git a/tests/test_eigen.cpp b/tests/test_eigen.cpp index 98711b08f..eeaceac1e 100644 --- a/tests/test_eigen.cpp +++ b/tests/test_eigen.cpp @@ -50,6 +50,16 @@ void reset_refs() { // Returns element 2,1 from a matrix (used to test copy/nocopy) double get_elem(Eigen::Ref m) { return m(2, 1); }; + +// Returns a matrix with 10*r + 100*c added to each matrix element (to help test that the matrix +// reference is referencing rows/columns correctly). +template Eigen::MatrixXd adjust_matrix(MatrixArgType m) { + Eigen::MatrixXd ret(m); + for (int c = 0; c < m.cols(); c++) for (int r = 0; r < m.rows(); r++) + ret(r, c) += 10*r + 100*c; + return ret; +} + test_initializer eigen([](py::module &m) { typedef Eigen::Matrix FixedMatrixR; typedef Eigen::Matrix FixedMatrixC; @@ -261,4 +271,10 @@ test_initializer eigen([](py::module &m) { // Also test a row-major-only no-copy const ref: m.def("get_elem_rm_nocopy", [](Eigen::Ref> &m) -> long { return m(2, 1); }, py::arg().noconvert()); + + // Issue #738: 1xN or Nx1 2D matrices were neither accepted nor properly copied with an + // incompatible stride value on the length-1 dimension--but that should be allowed (without + // requiring a copy!) because the stride value can be safely ignored on a size-1 dimension. + m.def("iss738_f1", &adjust_matrix &>, py::arg().noconvert()); + m.def("iss738_f2", &adjust_matrix> &>, py::arg().noconvert()); }); diff --git a/tests/test_eigen.py b/tests/test_eigen.py index c57ba2778..f14d62255 100644 --- a/tests/test_eigen.py +++ b/tests/test_eigen.py @@ -604,3 +604,13 @@ def test_sparse_signature(doc): assert doc(sparse_copy_c) == """ sparse_copy_c(arg0: scipy.sparse.csc_matrix[float32]) -> scipy.sparse.csc_matrix[float32] """ # noqa: E501 line too long + + +def test_issue738(): + from pybind11_tests import iss738_f1, iss738_f2 + + assert np.all(iss738_f1(np.array([[1., 2, 3]])) == np.array([[1., 102, 203]])) + assert np.all(iss738_f1(np.array([[1.], [2], [3]])) == np.array([[1.], [12], [23]])) + + assert np.all(iss738_f2(np.array([[1., 2, 3]])) == np.array([[1., 102, 203]])) + assert np.all(iss738_f2(np.array([[1.], [2], [3]])) == np.array([[1.], [12], [23]]))