trjhtr

I have a class named Matrix, this class inherits from Python's list. The class currently can perform matrix multiplication, adding rows using list.extend, add a row using list.append, and also transpose.

The emphasis is on using only built-in tools.

• I would like to know if this code can be made more efficient and readable.


• Also, if there are alternative techniques to produce the same result.


• a supplementary problem (more appropriate in StackOverflow) : I cannot use copy.deepcopy(A) with A a Matrix object. That's why i use res = Matrix(...) in the multiplication function.

Thanks.

Here is the code, break into three parts.

Global functions :

# Author: Arief Anbiya
# 11 February, 2018 

def dot_product(u, v):
 return sum([i*j for i,j in zip(u,v)]

def multiplication(M,N):
 assert type(M)==Matrix or type(N)==Matrix
 res = None;
 if type(M)==type(N)==Matrix and M.ncol()==N.nrow():
 res = Matrix([[0 for i in range(N.ncol())] for j in range(M.nrow())]);
 for i in range(M.nrow()):
 for j in range(N.ncol()):
 res[i][j] = dot_product(M[i], [N[k][j] for k in range(N.nrow())]);
 elif (type(M)==int or type(M)==float):
 res = Matrix([[N[j][i] for i in range(N.ncol())] for j in range(N.nrow())]);
 for i in range(res.nrow()):
 for j in range(res.ncol()):
 res[i][j] *= M;
 elif (type(N)==int or type(N)==float):
 res = Matrix([[M[j][i] for i in range(M.ncol())] for j in range(M.nrow())]);
 for i in range(res.nrow()):
 for j in range(res.ncol()):
 res[i][j] *= N; 
 else:
 raise TypeError("M and N should be either a compatible Matrix object, or a constant");
 return res

Class :

# Author: Arief Anbiya
# 11 February 2018
class Matrix(list):

 def __init__(self, the_list):
 super().__init__(the_list); 

 def nrow(self):
 m = len(self);
 return m
 def ncol(self):
 n = len(self[0]);
 return n
 def get_dim(self):
 return (self.nrow(),self.ncol())

 def __add__(self, M):
 res = Matrix(self);
 for row in range(self.nrow()):
 dumrow = [self[row][col] + M[row][col] for col in range(self.ncol())];
 res[row]=dumrow;
 return res
 def __mul__(self, M):
 return multiplication(self, M);
 def __rmul__(self,M):
 return multiplication(M, self);

 def add_rows(self, rows):
 super().extend(rows);
 def append(self, row):
 try: 
 sum(row); # Check if all numbers
 if len(row)<self.ncol():
 row.extend([0 for i in range(self.ncol()-len(row))]);
 elif len(row)>self.ncol():
 [row.pop() for i in range(len(row)-self.ncol())];
 super().append(row);
 except:
 raise AssertionError('Elements in row must be mumbers');

 def transpose(self):
 return Matrix([[row[i] for row in self] for i in range(self.ncol())]);

 def show(self):
 print("Printing matrix: ");
 for i in self:
 print(i);

Test for outputs :

A=Matrix([[1,2,3], [2,3,3]]);
A.show();
B = A+A;
B.show();
B.append([1,11,12])
B.show();
C = 3*B;
C.show();
D = A*B;
D.show()

This is an addendum to Stephen's detailed review

Instead of testing type of objects, use python's awesome isinstance function. You can then even have multiple valid types to check against.

assert type(M)==Matrix or type(N)==Matrix
(type(N)==int or type(N)==float)

will simply be:

assert isinstance(M, Matrix) or isinstace(N, Matrix)
isinstance(N, (int, float))

Once you have checked that both $ M $ and $ N $ are matrices and have computed their multiplication results, you do not need another if-else block. Just return the result as soon as computed.

Later, store the matrix in one variable and the scalar number in other, and use a single iteration to compute $ lambda cdot A $.

When creating a null matrix in the following statement:

res = Matrix([[0 for i in range(N.ncol())] for j in range(M.nrow())])

Make use of the $ * $ operator on lists:

res = Matrix([[0] * N.ncol for _ in range(M.nrow)]

A rewrite, from my perspective would be:

def multiplication(M, N):
 assert isinstance(M, (Matrix, int float)) and isinstace(N, (Matrix, int, float))
 if isinstace(M, Matrix) and isinstace(N, Matrix):
 if M.ncol != N.nrow
 raise TypeError("M and N should be either a compatible Matrix object, or a constant")
 res = Matrix([[0] * N.ncol] * M.nrow)
 for i in range(M.nrow()):
 for j in range(N.ncol):
 res[i][j] = dot_product(M[i], [N[k][j] for k in range(N.nrow)])
 return res
 left, right = (M, N) if isinstace(M, Matrix) else (N, M)
 res = Matrix(
 [
 [left[j][i] * right for i in range(N.ncol)]
 for j in range(N.nrow)
 ]
 )
 return res

You can have an additional check in the above if you wish to confirm whether left is also of type Matrix or not.

This review will focus on using Python more effectively. I will pull some examples from your Matrix class, and show those recast in a more Pythonic fashion.

Pep8

But before we start, you should consider formatting your code in accordance with pep8. This is important when sharing code, as the consistent style makes it much easier for other programmers to read your code. There are various tools available to assist in making the code pep8 compliant. I use the PyCharm IDE which will show pep8 violations right in the editor.

No Semi Colons

On the same front, lose the semi colons, they are very rarely (never really) needed in Python.

Iterate directly

Python has many great ways to iterate. In general if you find yourself looping on an integer, like in the following code, there is a good chance you are doing it wrong. It is generally better to iterate on the data structure itself. This example:

def __add__(self, M):
 res = Matrix(self);
 for row in range(self.nrow()):
 dumrow = [self[row][col] + M[row][col] for col in
 range(self.ncol())];
 res[row] = dumrow;
 return res

Can be reduced to:

def __add__(self, M):
 return Matrix([[sum(x) for x in zip(*rows)] for rows in zip(self, M)])

Note that the lengths of the rows or columns are not even looked at in this code. The underlying iterators take care of those details.

zip() is transpose

This hand coded transpose:

def transpose(self):
 return Matrix(
 [[row[i] for row in self] for i in range(self.ncol())]); 

can be recast to:

@property
def transpose(self):
 return Matrix(map(list, zip(*self))) 

zip() does the transpose, and the map(list(...)) converts the tuples from zip to lists. And...

@property make for a cleaner interface

Instead of:

def nrow(self):
 m = len(self);
 return m

Consider this instead:

@property
def nrow(self):
 return len(self)

It does two things:

1. Removes unneeded intermediate values


2. Use the @property decorator to allow calculated value to be returned without the need for the (). This can provide a cleaner api.

Full Listing

class Matrix(list):

 def __init__(self, the_list):
 super().__init__(the_list)

 @property
 def nrow(self):
 return len(self)

 @property
 def ncol(self):
 return len(self[0])

 def get_dim(self):
 return self.nrow, self.ncol

 def __add__(self, M):
 return Matrix(
 [[sum(x) for x in zip(*rows)] for rows in zip(self, M)])

 def __mul__(self, M):
 return multiplication(self, M)

 def __rmul__(self, M):
 return multiplication(M, self)

 def add_rows(self, rows):
 super().extend(rows)

 def append(self, row):
 try:
 sum(row) # Check if all numbers
 if len(row) < self.ncol:
 row.extend([0] * (self.ncol - len(row)))
 elif len(row) > self.ncol:
 [row.pop() for i in range(len(row) - self.ncol)]
 super().append(row)
 except:
 raise AssertionError('Elements in row must be mumbers')

 @property
 def transpose(self):
 return Matrix(map(list, zip(*self)))

 def show(self):
 print("Printing matrix: ")
 for i in self:
 print(i)