1
\$\begingroup\$

How can I refactor this code to generalize the argument parsing? I feel like this code can be better but I can't find a way. The usage syntax of the index operator is similar to the NumPy indexing syntax.

    def __getitem__(self, arg):
    """
    [] operator to index elements in the matrix
    Usage: 
    >> mat[0] returns first row
    >> mat[3, :] return the 4th row
    >> mat[:, 7] return the 8th column
    >> mat[0, 0] returns first element
    """
    if isinstance(arg, int):
        return self.matrix[arg]
    elif isinstance(arg, tuple):
        if isinstance(arg[0], int) and isinstance(arg[1], int):
            y, x = arg
            return self.matrix[x][y]
        elif isinstance(arg[0], slice) and isinstance(arg[1], int):
            return self.get_column(arg[1])
        elif isinstance(arg[0], int) and isinstance(arg[1], slice):
            return self.get_column(arg[1])
    else :
        raise TypeError('Invalid indexing arguments type', arg)

Also, how can I make a setitem function that uses the [] operator to set columns without rewriting the getitem function inside it? Here is my current implementation without the possibility of setting a column.

    def __setitem__(self, arg, value):
    if isinstance(value, Fraction):
        self[arg] = value
    elif isinstance(value, list) and all(isinstance(elem, Fraction) for elem in value):
        self[arg] = value
    else:
        raise TypeError('Invalid value type', value)
\$\endgroup\$
1
\$\begingroup\$

Before improving the code it would be worth fixing the following problems.

  1. The docstring says:

    >> mat[3, :] return the 4th row
    

    but the implementation tries to return a column:

    elif isinstance(arg[0], int) and isinstance(arg[1], slice):
        return self.get_column(arg[1])
    

    but since arg[1] is a slice object, this will fail in some way. (I can't say exactly how since you didn't show us the code for get_column.)

  2. The indexing is inconsistent. As it says in the docstring:

    >> mat[3, :] return the 4th row
    

    but to get the first element of the fourth row, you have to write:

    mat[0, 3]
    

    because the indexes get swapped:

    y, x = arg
    return self.matrix[x][y]
    

    This is contrary to the way indexing works in NumPy and so seems very likely to lead to confusion.

  3. If you pass a tuple of length 1 then this causes an IndexError:

    >>> mat[:,]
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
      File "cr189408.py", line 21, in __getitem__
        elif isinstance(arg[0], slice) and isinstance(arg[1], int):
    IndexError: tuple index out of range
    
  4. If you pass a pair of slices then this is ignored and __getitem__ returns None.

    >>> mat[:,:] is None
    True
    

    In NumPy this would return the whole matrix.

  5. The code only checks whether the argument is a slice or not, and if it is a slice, it assumes that the whole row or column is required. This is right if a bare : was passed, but misleading if some other slice was passed. In NumPy mat[0:2, 0] gets the first two element of column 0, but the code in the post returns the whole of column 0.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.