Repository: sympy/sympy
Pull request: Fixed wrapper for NumPy matrices so SymPy iteration and indexing works correctly
#### References to other Issues or PRs
Fixes #17522

#### Brief description of what is fixed or changed
Fixed wrapper for NumPy matrices so SymPy iteration and indexing works correctly.

#### Other comments


#### Release Notes
formatted correctly. -->

<!-- BEGIN RELEASE NOTES -->
* matrices
  * Fixed wrapper for NumPy matrices so SymPy iteration and indexing works correctly.
<!-- END RELEASE NOTES -->


Problem statement:
_MatrixWrapper does not wrap iteration of numpy matrix properly
The `_MatrixWrapper` class should make it possible to wrap a numpy matrix and use it like a SymPy Matrix but iteration does not work properly:
```julia
In [1]: from numpy import matrix                                                                                                  

In [2]: from sympy.matrices.common import _MatrixWrapper                                                                          

In [3]: M = matrix([[1, 2], [3, 4]])                                                                                              

In [4]: _MatrixWrapper(M)                                                                                                         
Out[4]: <sympy.matrices.common._MatrixWrapper at 0x119aa55f8>

In [5]: list(_MatrixWrapper(M))                                                                                                   
Out[5]: [matrix([[1, 2]]), matrix([[3, 4]])]

In [6]: M = Matrix([[1, 2], [3, 4]])                                                                                              

In [7]: list(M)                                                                                                                   
Out[7]: [1, 2, 3, 4]
```

Hints from issue discussion:
numpy array iteration scheme is different.
I don't think that `_MatrixWrapper` had intended to change that
but I have made the diff like below.

```
diff --git a/sympy/matrices/common.py b/sympy/matrices/common.py
index 881dc8a532..390b5b42d3 100644
--- a/sympy/matrices/common.py
+++ b/sympy/matrices/common.py
@@ -19,6 +19,7 @@
 from sympy.core.singleton import S
 from sympy.core.symbol import Symbol
 from sympy.core.sympify import sympify
+from sympy.external.importtools import import_module
 from sympy.functions import Abs
 from sympy.simplify import simplify as _simplify
 from sympy.utilities.exceptions import SymPyDeprecationWarning
@@ -26,6 +27,8 @@
 from sympy.utilities.misc import filldedent


+numpy = import_module('numpy')
+
 class MatrixError(Exception):
     pass

@@ -2476,6 +2479,12 @@ def __getattr__(self, attr):
     def __getitem__(self, key):
         return self.mat.__getitem__(key)

+    def __iter__(self):
+        mat = self.mat
+        if numpy and isinstance(mat, numpy.ndarray):
+            return mat.flat
+        return mat.__iter__()
+

 def _matrixify(mat):
     """If `mat` is a Matrix or is matrix-like,
```

But I don't think that it's enough to patch, since `_MatrixWrapper` also have a broken behavior of `len`

Commit message: added _MatrixWrapperRowIndexing

Patch to review:
diff --git a/sympy/matrices/common.py b/sympy/matrices/common.py
index 996a51937866..5a9bdf5b32d0 100644
--- a/sympy/matrices/common.py
+++ b/sympy/matrices/common.py
@@ -8,6 +8,7 @@
 
 from collections import defaultdict
 from inspect import isfunction
+from itertools import chain
 
 from sympy.assumptions.refine import refine
 from sympy.core import SympifyError, Add
@@ -2592,11 +2593,11 @@ def shape(self):
 
 
 class _MatrixWrapper(object):
-    """Wrapper class providing the minimum functionality
-    for a matrix-like object: .rows, .cols, .shape, indexability,
-    and iterability.  CommonMatrix math operations should work
-    on matrix-like objects.  For example, wrapping a numpy
-    matrix in a MatrixWrapper allows it to be passed to CommonMatrix.
+    """Wrapper class providing the minimum functionality for a matrix-like
+    object: .rows, .cols, .shape, indexability, and iterability. CommonMatrix
+    math operations should work on matrix-like objects. This one is intended for
+    matrix-like objects which use the same indexing format as SymPy with respect
+    to returning matrix elements instead of rows for non-tuple indexes.
     """
     is_MatrixLike = True
 
@@ -2613,6 +2614,37 @@ def __getitem__(self, key):
         return self.mat.__getitem__(key)
 
 
+class _MatrixWrapperRowIndexing(object):
+    """Wrapper class providing the minimum functionality for a matrix-like
+    object: .rows, .cols, .shape, indexability, and iterability. CommonMatrix
+    math operations should work on matrix-like objects. This one is intended for
+    matrix-like objects which return entire rows when indexed by a non-tuple
+    index. For example, wrapping a numpy matrix in a MatrixWrapper allows it to
+    be passed to CommonMatrix.
+    """
+    is_MatrixLike = True
+
+    def __init__(self, mat, shape=None):
+        self.mat = mat
+        self.rows, self.cols = mat.shape if shape is None else shape
+
+    def __getattr__(self, attr):
+        """Most attribute access is passed straight through
+        to the stored matrix"""
+        return getattr(self.mat, attr)
+
+    def __getitem__(self, key):
+        if isinstance(key, tuple):
+            return self.mat.__getitem__(key)
+
+        return self.mat.__getitem__((key // self.rows, key % self.cols))
+
+    def __iter__(self): # supports numpy.matrix and numpy.array
+        mat = self.mat
+        cols = self.cols
+
+        return iter(mat[r, c] for r in range(self.rows) for c in range(cols))
+
 def _matrixify(mat):
     """If `mat` is a Matrix or is matrix-like,
     return a Matrix or MatrixWrapper object.  Otherwise
@@ -2621,7 +2653,11 @@ def _matrixify(mat):
         return mat
     if hasattr(mat, 'shape'):
         if len(mat.shape) == 2:
-            return _MatrixWrapper(mat)
+            try: # test indexing behavior of matrix-like object
+                e = mat[0][0]
+                return _MatrixWrapperRowIndexing(mat)
+            except:
+                return _MatrixWrapper(mat)
     return mat
 
 


Code review comments:
diff_hunk: @@ -2621,7 +2653,11 @@ def _matrixify(mat):
         return mat
     if hasattr(mat, 'shape'):
         if len(mat.shape) == 2:
-            return _MatrixWrapper(mat)
+            try: # test indexing behavior of matrix-like object
+                e = mat[0][0]
+                return _MatrixWrapperRowIndexing(mat)
+            except:
+                return _MatrixWrapper(mat)
path: sympy/matrices/common.py
text: @user1:
Bare except should not be used