Mutable 2d Vector classs

Question

I create a 2dVector class to use a position coordinates in a pygame application and maybe in the future as part of a physics engine. Does this implementation has any huge downsides? Is the type hinting ok?

import math
import random
from numbers import Real
from typing import Union, Sequence, cast

VectorType = Sequence[Real]


class Vector(VectorType):
    x: Real
    y: Real
    rotation: Real
    magnitude: Real

    def __init__(self, x: Union[VectorType, Real], y: Real = None) -> None:
        if y is None:
            x, y = cast(VectorType, x)
        self.x, self.y = cast(Real, x), y

    def __repr__(self):
        return f"{self.__class__.__name__}{self.x, self.y}"

    def __len__(self) -> int:
        return 2

    def __getitem__(self, i: int) -> Real:
        return (self.x, self.y)[i]

    def __add__(self, other: VectorType):
        return Vector(self.x + other[0], self.y + other[1])

    def __radd__(self, other):
        return Vector(self.x + other[0], self.y + other[1])

    def __iadd__(self, other: VectorType):
        self.x += other[0]
        self.y += other[1]
        return self

    def __sub__(self, other: VectorType):
        return Vector(self.x - other[0], self.y - other[1])

    def __rsub__(self, other: VectorType):
        return Vector(other[0] - self.x, other[1] - self.y)

    def __isub__(self, other: VectorType):
        self.x -= other[0]
        self.y -= other[1]
        return self

    def __mul__(self, other: Real) -> Vector:
        return Vector(self.x * other, self.y * other)

    def __rmul__(self, other: Real):
        return Vector(self.x * other, self.y * other)

    def __matmul__(self, other: VectorType):
        return Vector(self.x * other[0], self.y * other[1])

    def __rmatmul__(self, other):
        return Vector(other[0] * self.x, other[1] * self.y)

    def __truediv__(self, other: Union[VectorType, Real]) -> Vector:
        if isinstance(other, Sequence):
            return Vector(self.x / other[0], self.y / other[1])
        return Vector(self.x / other, self.y / other)

    def __rtruediv__(self, other: VectorType) -> Vector:
        self.x /= other
        self.y /= other
        return self

    def __round__(self, n=None) -> Vector:
        return Vector(round(self.x, n), round(self.x, n))

    def change_rotation(self, rotation: float) -> Vector:
        v = Vector(self.x, self.y)
        v.rotation += rotation
        return v

    @classmethod
    def random(cls, x_range=(0, 1), y_range=(0, 1)):
        return cls(random.uniform(*x_range), random.uniform(*y_range))

    @classmethod
    def from_polar(cls, rotation: Real, magnitude: Real):
        return Vector(math.cos(rotation) * magnitude, math.sin(rotation) * magnitude)

    def normalize(self):
        f = self.magnitude
        self.x *= f
        self.y *= f
        return self

    @property
    def magnitude(self) -> Real:
        return (self.x * self.x + self.y * self.y) ** 0.5

    @magnitude.setter
    def magnitude(self, value: Real):
        f = value / self.magnitude
        self.x *= f
        self.y *= f

    @property
    def rotation(self):
        return math.atan2(self.x, self.y)

    @rotation.setter
    def rotation(self, value):
        m = self.magnitude
        self.x = math.sin(value)
        self.y = math.cos(value)
        self.magnitude = m

    @property
    def rounded(self):
        return round(self.x), round(self.y)

Here is the code on github (with a few additions)

Answer 1

1. There are no docstrings.

2. I would suggest reconsidering the mutability of the vectors. The advantages of mutability are:

   a. You get to reuse the original storage for the original vector when you make a change, avoiding a couple of memory management operations.

   b. You save a couple of characters when writing some update operations, for example you can write v.normalize() instead of, say, v = v.normalized().

   But the disadvantage of mutability is that it is harder to reason about operations. If you see a function that takes some vectors, let's say

   def collide(p: VectorType, q: VectorType, r: Real) -> Bool:
       "Return True if p and q are separated by no more than r."
   

   is it safe to pass p=sprite.position, or do you need to make a copy? With mutability, then for all you know, the function is implemented like this:

   p -= q
   return p.magnitude <= r
   

   So in a world where vectors are mutable, you have to worry about this possibility, and perhaps document for each function which arguments get mutated and which don't. If vectors are immutable, then all this is unnecessary and reasoning about the behaviour of code is easier.

   Immutability would avoid the need to implement the __iadd__ and __isub__ methods. Also, you could inherit from tuple and save some memory. See this vector class on github for some implementation ideas.

3. The __truediv__ method has different behaviour based on the type of the argument. This is inconsistent with the behaviour of __mul__ and __rmul__ and __rtruediv__, none of which have this behaviour. Also, dispatching based on the type makes it harder to read and reason about code. If you know that v is a vector and then you read the expression v / a, you can't immediately deduce that a must be a scalar as you could in the case of v * a.

   I guess that you added the variant operation because of some use case. But nonetheless I would recommend giving the variant operation a different method name, for example scaled.

4. In __radd__ the operations should be reversed, with the other values on the left (just in case they have unusual implementations of __add__ method):

   def __radd__(self, other):
       return Vector(other[0] + self.x, other[1] + self.y)
   

   Similarly for __rmul__.

5. There's no checking that the other vector has the right length. There's nothing stopping me from writing:

   Vector(1, 2) + [3, 4, 5]
   

   which ought to be an error.

6. In __repr__ I suggest writing type(self) rather than self.__class__, just as you would write len(s) rather than s.__len__().

7. In from_polar you should call cls rather than Vector, to support subclasses.

8. normalize has the operations the wrong way round: you need to divide by the magnitude, not multiply. But it would be simpler to write self.magnitude = 1.

9. In magnitude it would be clearer to call the argument length or magnitude rather than value.

10. In magnitude, consider writing math.hypot(self.x, self.y).

11. In rotation it would be clearer to call the argument theta or angle rather than value.