From 8c773add6468fc6da6b718f983fbe9008ad440a2 Mon Sep 17 00:00:00 2001
From: StephenGemin <45926479+StephenGemin@users.noreply.github.com>
Date: Wed, 26 Jun 2019 01:02:35 -0400
Subject: [PATCH 1/4] Update matrix_operation.py

1. Adding error checks for integer inputs
2. Adding error checks for matrix operations where size requirements do not match up
3. Added matrix subtraction function
4. included error check so only integer is passed into identity function
---
 matrix/matrix_operation.py | 122 +++++++++++++++++++++++++++----------
 1 file changed, 91 insertions(+), 31 deletions(-)

diff --git a/matrix/matrix_operation.py b/matrix/matrix_operation.py
index dd7c01582681..81f2bdcaf832 100644
--- a/matrix/matrix_operation.py
+++ b/matrix/matrix_operation.py
@@ -1,64 +1,123 @@
-from __future__ import print_function
+"""
+function based version of matrix operations, which are just 2D arrays
+"""
+
 
 def add(matrix_a, matrix_b):
-    rows = len(matrix_a)
-    columns = len(matrix_a[0])
-    matrix_c = []
-    for i in range(rows):
-        list_1 = []
-        for j in range(columns):
-            val = matrix_a[i][j] + matrix_b[i][j]
-            list_1.append(val)
-        matrix_c.append(list_1)
-    return matrix_c
-
-def scalarMultiply(matrix , n):
+    if _check_not_integer(matrix_a) & _check_not_integer(matrix_b):
+        rows, cols = _verify_matrix_sizes(matrix_a, matrix_b)
+        matrix_c = []
+        for i in range(rows[0]):
+            list_1 = []
+            for j in range(cols[0]):
+                val = matrix_a[i][j] + matrix_b[i][j]
+                list_1.append(val)
+            matrix_c.append(list_1)
+        return matrix_c
+
+
+def subtract(matrix_a, matrix_b):
+    if _check_not_integer(matrix_a) & _check_not_integer(matrix_b):
+        rows, cols = _verify_matrix_sizes(matrix_a, matrix_b)
+        matrix_c = []
+        for i in range(rows[0]):
+            list_1 = []
+            for j in range(cols[0]):
+                val = matrix_a[i][j] - matrix_b[i][j]
+                list_1.append(val)
+            matrix_c.append(list_1)
+        return matrix_c
+
+
+def scalar_multiply(matrix, n):
     return [[x * n for x in row] for row in matrix]
 
+
 def multiply(matrix_a, matrix_b):
-    matrix_c = []
-    n = len(matrix_a)
-    for i in range(n):
-        list_1 = []
-        for j in range(n):
-            val = 0
-            for k in range(n):
-                val = val + matrix_a[i][k] * matrix_b[k][j]
-            list_1.append(val)
-        matrix_c.append(list_1)
-    return matrix_c
+    if _check_not_integer(matrix_a) & _check_not_integer(matrix_b):
+        matrix_c = []
+        rows, cols = _verify_matrix_sizes(matrix_a, matrix_b)
+
+        if cols[0] != rows[1]:
+            raise ValueError(f'Cannot multiply matrix of dimensions ({rows[0]},{cols[0]}) '
+                             f'and ({rows[1]},{cols[1]})')
+        for i in range(rows[0]):
+            list_1 = []
+            for j in range(cols[1]):
+                val = 0
+                for k in range(cols[1]):
+                    val = val + matrix_a[i][k] * matrix_b[k][j]
+                list_1.append(val)
+            matrix_c.append(list_1)
+        return matrix_c
+
 
 def identity(n):
+    """
+    :param n: dimension for nxn matrix
+    :type n: int
+    :return: Identity matrix of shape [n, n]
+    """
+    n = int(n)
     return [[int(row == column) for column in range(n)] for row in range(n)] 
 
+
 def transpose(matrix):
-    return map(list , zip(*matrix))
+    return map(list, zip(*matrix))
+
 
 def minor(matrix, row, column):
     minor = matrix[:row] + matrix[row + 1:]
     minor = [row[:column] + row[column + 1:] for row in minor]
     return minor
 
+
 def determinant(matrix):
-    if len(matrix) == 1: return matrix[0][0]
+    if len(matrix) == 1:
+        return matrix[0][0]
     
     res = 0
     for x in range(len(matrix)):
-        res += matrix[0][x] * determinant(minor(matrix , 0 , x)) * (-1) ** x
+        res += matrix[0][x] * determinant(minor(matrix, 0, x)) * (-1) ** x
     return res
 
+
 def inverse(matrix):
     det = determinant(matrix)
-    if det == 0: return None
+    if det == 0:
+        return None
 
     matrixMinor = [[] for _ in range(len(matrix))]
     for i in range(len(matrix)):
         for j in range(len(matrix)):
-            matrixMinor[i].append(determinant(minor(matrix , i , j)))
+            matrixMinor[i].append(determinant(minor(matrix, i, j)))
     
     cofactors = [[x * (-1) ** (row + col) for col, x in enumerate(matrixMinor[row])] for row in range(len(matrix))]
     adjugate = transpose(cofactors)
-    return scalarMultiply(adjugate , 1/det)
+    return scalar_multiply(adjugate, 1/det)
+
+
+def _check_not_integer(matrix):
+    try:
+        rows = len(matrix)
+        cols = len(matrix)
+        return True
+    except TypeError:
+        raise TypeError("Cannot input an integer value, it must be a matrix")
+
+
+def _shape(matrix):
+    return list((len(matrix), len(matrix[0])))
+
+
+def _verify_matrix_sizes(matrix_a, matrix_b):
+    shape = _shape(matrix_a)
+    shape += _shape(matrix_b)
+    if shape[0] != shape[2] or shape[1] != shape[3]:
+        raise ValueError(f"operands could not be broadcast together with shape "
+                         f"({shape[0], shape[1]}), ({shape[2], shape[3]})")
+    return [shape[0], shape[2]], [shape[1], shape[3]]
+
 
 def main():
     matrix_a = [[12, 10], [3, 9]]
@@ -68,9 +127,10 @@ def main():
     print('Add Operation, %s + %s = %s \n' %(matrix_a, matrix_b, (add(matrix_a, matrix_b))))
     print('Multiply Operation, %s * %s = %s \n' %(matrix_a, matrix_b, multiply(matrix_a, matrix_b)))
     print('Identity:  %s \n' %identity(5))
-    print('Minor of %s = %s \n' %(matrix_c, minor(matrix_c , 1 , 2)))
+    print('Minor of %s = %s \n' %(matrix_c, minor(matrix_c, 1, 2)))
     print('Determinant of %s = %s \n' %(matrix_b, determinant(matrix_b)))
     print('Inverse of %s = %s\n'%(matrix_d, inverse(matrix_d)))
 
+
 if __name__ == '__main__':
     main()

From 2d5b2a2106ac94004337888ec6c414b079c743ba Mon Sep 17 00:00:00 2001
From: StephenGemin <45926479+StephenGemin@users.noreply.github.com>
Date: Wed, 26 Jun 2019 01:04:26 -0400
Subject: [PATCH 2/4] Create test_matrix_operation.py

---
 matrix/test_matrix_operation.py | 101 ++++++++++++++++++++++++++++++++
 1 file changed, 101 insertions(+)
 create mode 100644 matrix/test_matrix_operation.py

diff --git a/matrix/test_matrix_operation.py b/matrix/test_matrix_operation.py
new file mode 100644
index 000000000000..f0c07582c3e1
--- /dev/null
+++ b/matrix/test_matrix_operation.py
@@ -0,0 +1,101 @@
+"""
+Testing here assumes that numpy is ALWAYS correct!!!!
+
+If running from PyCharm you can place the following line in "Additional Arguments" for the pytest run configuration
+-vv -m mat_ops -p no:cacheprovider
+"""
+
+# standard libraries
+import numpy as np
+import pytest
+
+# Custom/local libraries
+from matrix import matrix_operation as matop
+
+mat_a = [[12, 10], [3, 9]]
+mat_b = [[3, 4], [7, 4]]
+mat_c = [[3, 0, 2], [2, 0, -2], [0, 1, 1]]
+mat_d = [[3, 0, 2], [2, 0, -2], [0, 1, 1]]
+mat_e = [[3, 0, 2], [2, 0, -2], [0, 1, 1], [2, 0, -2]]
+mat_f = [1]
+mat_h = [2]
+
+
+@pytest.mark.mat_ops
+@pytest.mark.parametrize(('mat1', 'mat2'), [(mat_a, mat_b), (mat_c, mat_d), (mat_d, mat_e),
+                                            (mat_f, mat_h)])
+def test_addition(mat1, mat2):
+    # Catch when user enters a single integer value rather than a mat
+    # Check for known errors
+    if (np.array(mat1)).shape < (2, 2) or (np.array(mat2)).shape < (2, 2):
+        with pytest.raises(TypeError):
+            assert matop.add(mat1, mat2)
+    elif (np.array(mat1)).shape == (np.array(mat2)).shape:
+        act = (np.array(mat1) + np.array(mat2)).tolist()
+        theo = matop.add(mat1, mat2)
+        assert theo == act
+    else:
+        # matrices have different dimensions
+        # Check for known errors
+        with pytest.raises(ValueError):
+            assert matop.add(mat1, mat2)
+
+
+@pytest.mark.mat_ops
+@pytest.mark.parametrize(('mat1', 'mat2'), [(mat_a, mat_b), (mat_c, mat_d), (mat_d, mat_e),
+                                            (mat_f, mat_h)])
+def test_subtraction(mat1, mat2):
+    # Catch when user enters a single integer value rather than a mat
+    # Check for known errors
+    if (np.array(mat1)).shape < (2, 2) or (np.array(mat2)).shape < (2, 2):
+        with pytest.raises(TypeError):
+            assert matop.add(mat1, mat2)
+    elif (np.array(mat1)).shape == (np.array(mat2)).shape:
+        act = (np.array(mat1) - np.array(mat2)).tolist()
+        theo = matop.subtract(mat1, mat2)
+        assert theo == act
+    else:
+        # matrices have different dimensions
+        # Check for known errors
+        with pytest.raises(ValueError):
+            assert matop.subtract(mat1, mat2)
+
+
+@pytest.mark.mat_ops
+@pytest.mark.parametrize(('mat1', 'mat2'), [(mat_a, mat_b), (mat_c, mat_d), (mat_d, mat_e),
+                                            (mat_f, mat_h)])
+def test_multiplication(mat1, mat2):
+    # Catch when user enters a single integer value rather than a mat
+    # Check for known errors
+    if (np.array(mat1)).shape < (2, 2) or (np.array(mat2)).shape < (2, 2):
+        with pytest.raises(TypeError):
+            assert matop.add(mat1, mat2)
+    elif (np.array(mat1)).shape == (np.array(mat2)).shape:
+        act = (np.matmul(mat1, mat2)).tolist()
+        theo = matop.multiply(mat1, mat2)
+        assert theo == act
+    else:
+        # matrices have different dimensions
+        # Check for known errors
+        with pytest.raises(ValueError):
+            assert matop.subtract(mat1, mat2)
+
+
+@pytest.mark.mat_ops
+def test_scalar_multiply():
+    act = (3.5 * np.array(mat_a)).tolist()
+    theo = matop.scalar_multiply(mat_a, 3.5)
+    assert theo == act
+
+
+@pytest.mark.mat_ops
+def test_identity():
+    act = (np.identity(5)).tolist()
+    theo = matop.identity(5)
+    assert theo == act
+
+
+@pytest.mark.map_ops
+@pytest.mark.parametrize
+def test_transpose():
+    act = np.transpose()
\ No newline at end of file

From c969230cf159b775a02b19420fa0691911654678 Mon Sep 17 00:00:00 2001
From: StephenGemin <45926479+StephenGemin@users.noreply.github.com>
Date: Wed, 26 Jun 2019 23:03:09 -0400
Subject: [PATCH 3/4] Update matrix_ops and Add Test Cases

1. Included error checks in matrix operation.  There were some cases where the functions would not work correctly.

2. PEP8 changes to matrix_operations.py

3. added test cases for matrix operations using pytest.
---
 matrix/matrix_operation.py                  | 26 ++++++----
 matrix/tests/pytest.ini                     |  3 ++
 matrix/{ => tests}/test_matrix_operation.py | 55 ++++++++++++---------
 3 files changed, 53 insertions(+), 31 deletions(-)
 create mode 100644 matrix/tests/pytest.ini
 rename matrix/{ => tests}/test_matrix_operation.py (61%)

diff --git a/matrix/matrix_operation.py b/matrix/matrix_operation.py
index 81f2bdcaf832..b32a4dcf7af3 100644
--- a/matrix/matrix_operation.py
+++ b/matrix/matrix_operation.py
@@ -4,7 +4,7 @@
 
 
 def add(matrix_a, matrix_b):
-    if _check_not_integer(matrix_a) & _check_not_integer(matrix_b):
+    if _check_not_integer(matrix_a) and _check_not_integer(matrix_b):
         rows, cols = _verify_matrix_sizes(matrix_a, matrix_b)
         matrix_c = []
         for i in range(rows[0]):
@@ -17,7 +17,7 @@ def add(matrix_a, matrix_b):
 
 
 def subtract(matrix_a, matrix_b):
-    if _check_not_integer(matrix_a) & _check_not_integer(matrix_b):
+    if _check_not_integer(matrix_a) and _check_not_integer(matrix_b):
         rows, cols = _verify_matrix_sizes(matrix_a, matrix_b)
         matrix_c = []
         for i in range(rows[0]):
@@ -34,7 +34,7 @@ def scalar_multiply(matrix, n):
 
 
 def multiply(matrix_a, matrix_b):
-    if _check_not_integer(matrix_a) & _check_not_integer(matrix_b):
+    if _check_not_integer(matrix_a) and _check_not_integer(matrix_b):
         matrix_c = []
         rows, cols = _verify_matrix_sizes(matrix_a, matrix_b)
 
@@ -62,8 +62,16 @@ def identity(n):
     return [[int(row == column) for column in range(n)] for row in range(n)] 
 
 
-def transpose(matrix):
-    return map(list, zip(*matrix))
+def transpose(matrix, return_map=True):
+    if _check_not_integer(matrix):
+        if return_map:
+            return map(list, zip(*matrix))
+        else:
+            # mt = []
+            # for i in range(len(matrix[0])):
+            #     mt.append([row[i] for row in matrix])
+            # return mt
+            return [[row[i] for row in matrix] for i in range(len(matrix[0]))]
 
 
 def minor(matrix, row, column):
@@ -87,12 +95,12 @@ def inverse(matrix):
     if det == 0:
         return None
 
-    matrixMinor = [[] for _ in range(len(matrix))]
+    matrix_minor = [[] for _ in range(len(matrix))]
     for i in range(len(matrix)):
         for j in range(len(matrix)):
-            matrixMinor[i].append(determinant(minor(matrix, i, j)))
+            matrix_minor[i].append(determinant(minor(matrix, i, j)))
     
-    cofactors = [[x * (-1) ** (row + col) for col, x in enumerate(matrixMinor[row])] for row in range(len(matrix))]
+    cofactors = [[x * (-1) ** (row + col) for col, x in enumerate(matrix_minor[row])] for row in range(len(matrix))]
     adjugate = transpose(cofactors)
     return scalar_multiply(adjugate, 1/det)
 
@@ -100,7 +108,7 @@ def inverse(matrix):
 def _check_not_integer(matrix):
     try:
         rows = len(matrix)
-        cols = len(matrix)
+        cols = len(matrix[0])
         return True
     except TypeError:
         raise TypeError("Cannot input an integer value, it must be a matrix")
diff --git a/matrix/tests/pytest.ini b/matrix/tests/pytest.ini
new file mode 100644
index 000000000000..6f9933ac351f
--- /dev/null
+++ b/matrix/tests/pytest.ini
@@ -0,0 +1,3 @@
+[pytest]
+markers =
+    mat_ops: tests for matrix operations
\ No newline at end of file
diff --git a/matrix/test_matrix_operation.py b/matrix/tests/test_matrix_operation.py
similarity index 61%
rename from matrix/test_matrix_operation.py
rename to matrix/tests/test_matrix_operation.py
index f0c07582c3e1..8b81b65d0fc8 100644
--- a/matrix/test_matrix_operation.py
+++ b/matrix/tests/test_matrix_operation.py
@@ -1,13 +1,15 @@
 """
-Testing here assumes that numpy is ALWAYS correct!!!!
+Testing here assumes that numpy and linalg is ALWAYS correct!!!!
 
 If running from PyCharm you can place the following line in "Additional Arguments" for the pytest run configuration
 -vv -m mat_ops -p no:cacheprovider
 """
 
 # standard libraries
+import sys
 import numpy as np
 import pytest
+import logging
 
 # Custom/local libraries
 from matrix import matrix_operation as matop
@@ -15,49 +17,52 @@
 mat_a = [[12, 10], [3, 9]]
 mat_b = [[3, 4], [7, 4]]
 mat_c = [[3, 0, 2], [2, 0, -2], [0, 1, 1]]
-mat_d = [[3, 0, 2], [2, 0, -2], [0, 1, 1]]
+mat_d = [[3, 0, -2], [2, 0, 2], [0, 1, 1]]
 mat_e = [[3, 0, 2], [2, 0, -2], [0, 1, 1], [2, 0, -2]]
 mat_f = [1]
 mat_h = [2]
 
+logger = logging.getLogger()
+logger.level = logging.DEBUG
+stream_handler = logging.StreamHandler(sys.stdout)
+logger.addHandler(stream_handler)
+
 
 @pytest.mark.mat_ops
 @pytest.mark.parametrize(('mat1', 'mat2'), [(mat_a, mat_b), (mat_c, mat_d), (mat_d, mat_e),
                                             (mat_f, mat_h)])
 def test_addition(mat1, mat2):
-    # Catch when user enters a single integer value rather than a mat
-    # Check for known errors
     if (np.array(mat1)).shape < (2, 2) or (np.array(mat2)).shape < (2, 2):
         with pytest.raises(TypeError):
-            assert matop.add(mat1, mat2)
+            logger.info(f"\n\t{test_addition.__name__} returned integer")
+            matop.add(mat1, mat2)
     elif (np.array(mat1)).shape == (np.array(mat2)).shape:
+        logger.info(f"\n\t{test_addition.__name__} with same matrix dims")
         act = (np.array(mat1) + np.array(mat2)).tolist()
         theo = matop.add(mat1, mat2)
         assert theo == act
     else:
-        # matrices have different dimensions
-        # Check for known errors
         with pytest.raises(ValueError):
-            assert matop.add(mat1, mat2)
+            logger.info(f"\n\t{test_addition.__name__} with different matrix dims")
+            matop.add(mat1, mat2)
 
 
 @pytest.mark.mat_ops
 @pytest.mark.parametrize(('mat1', 'mat2'), [(mat_a, mat_b), (mat_c, mat_d), (mat_d, mat_e),
                                             (mat_f, mat_h)])
 def test_subtraction(mat1, mat2):
-    # Catch when user enters a single integer value rather than a mat
-    # Check for known errors
     if (np.array(mat1)).shape < (2, 2) or (np.array(mat2)).shape < (2, 2):
         with pytest.raises(TypeError):
-            assert matop.add(mat1, mat2)
+            logger.info(f"\n\t{test_subtraction.__name__} returned integer")
+            matop.subtract(mat1, mat2)
     elif (np.array(mat1)).shape == (np.array(mat2)).shape:
+        logger.info(f"\n\t{test_subtraction.__name__} with same matrix dims")
         act = (np.array(mat1) - np.array(mat2)).tolist()
         theo = matop.subtract(mat1, mat2)
         assert theo == act
     else:
-        # matrices have different dimensions
-        # Check for known errors
         with pytest.raises(ValueError):
+            logger.info(f"\n\t{test_subtraction.__name__} with different matrix dims")
             assert matop.subtract(mat1, mat2)
 
 
@@ -65,19 +70,18 @@ def test_subtraction(mat1, mat2):
 @pytest.mark.parametrize(('mat1', 'mat2'), [(mat_a, mat_b), (mat_c, mat_d), (mat_d, mat_e),
                                             (mat_f, mat_h)])
 def test_multiplication(mat1, mat2):
-    # Catch when user enters a single integer value rather than a mat
-    # Check for known errors
     if (np.array(mat1)).shape < (2, 2) or (np.array(mat2)).shape < (2, 2):
+        logger.info(f"\n\t{test_multiplication.__name__} returned integer")
         with pytest.raises(TypeError):
-            assert matop.add(mat1, mat2)
+            matop.add(mat1, mat2)
     elif (np.array(mat1)).shape == (np.array(mat2)).shape:
+        logger.info(f"\n\t{test_multiplication.__name__} meets dim requirements")
         act = (np.matmul(mat1, mat2)).tolist()
         theo = matop.multiply(mat1, mat2)
         assert theo == act
     else:
-        # matrices have different dimensions
-        # Check for known errors
         with pytest.raises(ValueError):
+            logger.info(f"\n\t{test_multiplication.__name__} does not meet dim requirements")
             assert matop.subtract(mat1, mat2)
 
 
@@ -95,7 +99,14 @@ def test_identity():
     assert theo == act
 
 
-@pytest.mark.map_ops
-@pytest.mark.parametrize
-def test_transpose():
-    act = np.transpose()
\ No newline at end of file
+@pytest.mark.mat_ops
+@pytest.mark.parametrize('mat', [mat_a, mat_b, mat_c, mat_d, mat_e, mat_f])
+def test_transpose(mat):
+    if (np.array(mat)).shape < (2, 2):
+        with pytest.raises(TypeError):
+            logger.info(f"\n\t{test_transpose.__name__} returned integer")
+            matop.transpose(mat)
+    else:
+        act = (np.transpose(mat)).tolist()
+        theo = matop.transpose(mat, return_map=False)
+        assert theo == act

From 0d146df04d96d21039ad20487237e43aaa2bdcd0 Mon Sep 17 00:00:00 2001
From: Stephen Gemin <45926479+StephenGemin@users.noreply.github.com>
Date: Fri, 19 Jul 2019 18:16:21 -0400
Subject: [PATCH 4/4] Update pytest.ini

Add carriage return to end of file
---
 matrix/tests/pytest.ini | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/matrix/tests/pytest.ini b/matrix/tests/pytest.ini
index 6f9933ac351f..8a978b56ef8b 100644
--- a/matrix/tests/pytest.ini
+++ b/matrix/tests/pytest.ini
@@ -1,3 +1,3 @@
 [pytest]
 markers =
-    mat_ops: tests for matrix operations
\ No newline at end of file
+    mat_ops: tests for matrix operations