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)

• Jean-François Fabre: Vector(round(self.x, n), round(self.x, n)) typo for Vector(round(self.x, n), round(self.y, n)) ?
• MegaIng: @Jean-FrançoisFabre Yes, glad you pointed that out. Would have been a pretty hard to catch bug.
• Jean-François Fabre: also in rotation you're assigning m to the magnitude, then assign it the other way round: it achieves nothing
• Jean-François Fabre: pretty nice code, that said
• MegaIng: @Jean-FrançoisFabre yes it does. It will set self.x and self.y to the correct values. (Multiply them with magnitude) , because self.magnitude = m invokes the property setter.

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.

• MegaIng: Do you think I need to add docstrings to all methods? Even __add__ or __getitem__?
• Gareth Rees: I wouldn't write docstrings for methods that are already adequately documented in the Python language reference. Unless there's something special or surprising about your implementation.
• MegaIng: Added suggestion 1 and 4-11 to github. To 2: I want to keep the mutability. It is quite useful. To 3: I want to keep the simple division by a scalar or a vector. I no it is a mess, I already thoughtabout overwriting __divmod__ and returning result, None, but that is better in my opinion. Any ideas?
• MegaIng: I mean overwriting __divmod__ isn't better
• Gareth Rees: The specification for __divmod__ says: "The __divmod__ method should be the equivalent to using __floordiv__ and __mod__; it should not be related to __truediv__." So overriding this method with something related to __truediv__ has the potential to lead to confusion and error. I suggest using another method name, for example scaled or scaled_down.