# Basic beginner's matrix class in Python

This is my first class. I've been working a while on it, some functions are future ideas and are right now just bare bones and intended to be worked on later. I'm really looking for a good review of what I have so far, I've never done this before so hopefully I'm off to a decent start. I tried to log my errors as well and handle them by skipping but annotating which values are trouble. Also first time using *args in my functions.

class Matrix():

def __init__(self, height, width):
self.rows = [[0]*width for i in range(height)]
self.height = height
self.width = width

def __str__(self):
s = "\n" + "\n".join([str(i) for i in [rows for rows in self.rows] ]) + "\n"
return s

def __repr__(self):
return (f'{self.__class__.__name__} ({self.height!r} , {self.width!r})')

def len(self):
return self.height * self.width

return

def __mul__(self, matrix2):
return

def remove(self, item):
return

def fill_matrix(self, fill_list):
index = 0
for i in range(len(self.rows)):
try:
for j in range(len(self.rows[i])):
self.rows[i][j] = fill_list[index]
index += 1
except IndexError:
print (f"Matrix not filled \nMatrix fill stopped at: row {i}, Column {j}")
break
return fill_list[index:]

log = []
for arg in args:
try:
arg_size = len(arg)
except TypeError:
log.append(f'Parameter must be sequence, skipped: {arg}')
continue

try:
if arg_size == 3:
self.rows[arg[0]] [arg[1]] = arg[2]
else:
log.append(f'Parameter has too little or too much data, skipped: {arg}')
except  IndexError:
log.append(f'Location parameters are out of range, skipped: {arg}')
except TypeError:
log.append(f'Location indicies must contain integral types, skipped: {arg}')

return log

myMat = Matrix(5,5)
overflow = myMat.fill_matrix([i for i in range(26)])

print(myMat)

Errors =  myMat.add_value((-1,3,500), (0,0,3),(51,5, 7), (1, 2, 667), [3,4,676], (1), (1,"a", 1), (1,1, "£"))
print(myMat)

• I know no one has really said anything about it, but I'm generalizing my fill function to fill partitions or whole thing – Anonymous3.1415 Oct 30 '18 at 18:09
• Is there any reason not to use NumPy instead of writing your own class? – Georgy Oct 30 '18 at 19:33
• @Georgy, yes, I'm trying to write this Matrix Object to increase my understanding of how to think in an Object Oriented manner and to see if my solution and approach so far is a good start – Anonymous3.1415 Oct 30 '18 at 20:28

1. There is no need for brackets in class Matrix(): .... It can be just class Matrix: .... See docs for class definition syntax.
2. In self.rows = [[0]*width for i in range(height)] you should replace i by _ that we usually use for throwaway variables.
3. In the same line 0 is a magic number. You could make it a default parameter:

def __init__(self, height, width, fill_value=0):
self.rows = [[fill_value] * width for _ in range(height)]


1. Regarding __str__:

def __str__(self):
s = "\n" + "\n".join([str(i) for i in [rows for rows in self.rows] ]) + "\n"
return s


Having those "\n" on both ends seems unnecessary. Just return the middle part and let user decide if he wants more blank lines between output.

2. [rows for rows in self.rows] is the same as self.rows. That leaves us:

def __str__(self):
s = "\n".join([str(i) for i in self.rows])
return s

3. i is a bad name. row is better.

4. Removing unnecessary s:

def __str__(self):
return "\n".join([str(row) for row in self.rows])

5. And making it more concise:

def __str__(self):
return "\n".join(map(str, self.rows))


1. Regarding __repr__:

def __repr__(self):
return (f'{self.__class__.__name__} ({self.height!r} , {self.width!r})')


The brackets are unnecessary here, and spacing is a bit weird. It should be like this:

return f'{self.__class__.__name__}({self.height!r}, {self.width!r})'

2. As you probably already know, with __repr__ you should be able to pass the returned string to Python interpreter so that it could recreate the object. But in your case information about the values in your matrix would be lost. What you return is just something like Matrix(5, 5). Would this be OK? Probably it would be better if the values would be returned as well, but then the current logic of the class won't allow you to recreate the matrix as you separated filling (fill_matrix) from initializing. Maybe it would be better to be able to do both at the same time?

1. len method should be called __len__.

2. As with current implementation its value is constant, you could use lru_cache(1) so to calculate it only once and then reuse the cached value:

from functools import lru_cache

...

@lru_cache(1)
def __len__(self) -> int:
return self.height * self.width

3. You have some not implemented yet methods. Instead of returning nothing:

def __add__(self, matrix2):
return


raise a NotImplementedError:

def __add__(self, matrix2):
raise NotImplementedError


1. Regarding fill_matrix:

def fill_matrix(self, fill_list):
index = 0
for i in range(len(self.rows)):
try:
for j in range(len(self.rows[i])):
self.rows[i][j] = fill_list[index]
index += 1
except IndexError:
print (f"Matrix not filled \nMatrix fill stopped at: row {i}, Column {j}")
break
return fill_list[index:]


Add a docstring. It is not clear at the first sight why this method should return anything. Something like:

def fill_matrix(self, fill_list):
"""
Fills matrix by fill_list
and returns what didn't fit in it.
"""

2. You are catching IndexError but this case seems impossible as indices are always taken from the ranges defined by shape of the matrix. You can remove it.

3. Why recalculating len(self.rows) and len(self.rows[i]) if you already have self.height and self.width?

4. The idea of iterating the index doesn't seem Pythonic. How about something like this instead:

from itertools import product

...

def fill_matrix(self, fill_list):
fill_values = iter(fill_list)

indices = product(range(self.height), range(self.width))

for (i, j), value in zip(indices, fill_values):
self.rows[i][j] = value

return fill_values


This can accept iterators or sequences and will return an iterator with "overflow". So if you need to use those leftovers, you get them like this:

myMat = Matrix(5, 5)
overflow = myMat.fill(range(30))
print(list(overflow))
>>> [25, 26, 27, 28, 29]

5. It would be better to rename fill_matrix as fill and its argument fill_list as values.

1. Regarding add_value. I think it's better to name it replace instead.

2. This method is too complicated and doesn't follow Single responsibility principle. Catch the errors outside, and use this method just for replacing one value. If you need to replace multiple values, then put the function call inside the loop. See this post for a better explanation. In the end it should be as simple as that:

def insert(self, row_index, column_index, value):
self.rows[row_index][column_index] = value


And you could call it, and catch and save the errors like this:

import traceback

...

values = [(-1, 3, 500),
(0, 0, 3),
(51, 5, 7),
(1, 2, 667),
[3, 4, 676],
(1),
[3, 4, 676, 123],
(1, "a", 1),
(1, 1, "£")]

errors = []

for value in values:
try:
myMat.insert(*value)
except (IndexError, TypeError):
errors.append(traceback.format_exc())

for error, value in zip(errors, bad_values):
print(f'Error occurred for value {value}:\n'
f'{error}')


And the output is:

Error occurred for value (51, 5, 7):
Traceback (most recent call last):
File "C:/Users/Georgy/SO_CR/main.py", line 82, in <module>
myMat.insert(*value)
File "C:/Users/Georgy/SO_CR/main.py", line 55, in insert
self.rows[row_index][column_index] = value
IndexError: list index out of range

Error occurred for value 1:
Traceback (most recent call last):
File "C:/Users/Georgy/SO_CR/main.py", line 82, in <module>
myMat.insert(*value)
TypeError: insert() argument after * must be an iterable, not int

Error occurred for value [3, 4, 676, 123]:
Traceback (most recent call last):
File "C:/Users/Georgy/SO_CR/main.py", line 82, in <module>
myMat.insert(*value)
TypeError: insert() takes 4 positional arguments but 5 were given

Error occurred for value (1, 'a', 1):
Traceback (most recent call last):
File "C:/Users/Georgy/SO_CR/main.py", line 82, in <module>
myMat.insert(*value)
File "C:/Users/Georgy/SO_CR/main.py", line 55, in insert
self.rows[row_index][column_index] = value
TypeError: list indices must be integers or slices, not str


You might be interested in Python logging module as well. Though, I find it strange that you decided to pay so much attention to catching the errors.

Some other notes:

• Don't forget about naming conventions. myMat should be my_matrix, Errors should be errors. See PEP 8: for more.
• Consider using type hints.

In the end your code could look like this:

from functools import lru_cache
from itertools import product
from typing import (Any,
Iterable,
Iterator)

class Matrix:
def __init__(self,
height: int,
width: int,
fill_value: Any = 0) -> None:
self.height = height
self.width = width
self.rows = [[fill_value] * width for _ in range(height)]

def __str__(self) -> str:
return "\n".join(map(str, self.rows))

def __repr__(self) -> str:
return f'{self.__class__.__name__}({self.height!r}, {self.width!r})'

@lru_cache(1)
def __len__(self) -> int:
return self.height * self.width

raise NotImplementedError

def __mul__(self, matrix2):
raise NotImplementedError

def remove(self, item):
raise NotImplementedError

def fill(self, values: Iterable[Any]) -> Iterator[Any]:
"""
Fills matrix by fill_values
and returns what didn't fit in it.
"""
values = iter(values)

indices = product(range(self.height), range(self.width))

for (i, j), value in zip(indices, values):
self.rows[i][j] = value

return values

def replace(self,
row_index: int,
column_index: int,
value: Any) -> None:
"""Replaces a value by given indices by a new one."""
self.rows[row_index][column_index] = value