Eigen: don't require conformability on length-1 dimensions

Fixes #738

The current check for conformability fails when given a 2D, 1xN or Nx1
input to a row-major or column-major, respectively, Eigen::Ref, leading
to a copy-required state in the type_caster, but this later failed
because the copy was also non-conformable because it had the same shape
and strides (because a 1xN or Nx1 is both F and C contiguous).

In such cases we can safely ignore the stride on the "1" dimension since
it'll never be used: only the "N" dimension stride needs to match the
Eigen::Ref stride, which both fixes the non-conformable copy problem,
but also avoids a copy entirely as long as the "N" dimension has a
compatible stride.

include/pybind11/eigen.h

@@ -79,11 +79,17 @@ template <bool EigenRowMajor> 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 <typename props> 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::inner_stride == Eigen::Dynamic || props::inner_stride == stride.inner() ||
-            (props::outer_stride == Eigen::Dynamic || props::outer_stride == stride.outer());
+                (EigenRowMajor ? cols : rows) == 1) &&
+            (props::outer_stride == Eigen::Dynamic || props::outer_stride == stride.outer() ||
+                (EigenRowMajor ? rows : cols) == 1);
     }
     operator bool() const { return conformable; }
 };

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<const Eigen::MatrixXd> 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 <typename MatrixArgType> 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<float, 5, 6, Eigen::RowMajor> FixedMatrixR;
     typedef Eigen::Matrix<float, 5, 6> 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<const Eigen::Matrix<long, -1, -1, Eigen::RowMajor>> &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<const Eigen::Ref<const Eigen::MatrixXd> &>, py::arg().noconvert());
+    m.def("iss738_f2", &adjust_matrix<const Eigen::Ref<const Eigen::Matrix<double, -1, -1, Eigen::RowMajor>> &>, py::arg().noconvert());
 });

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]]))