vpython / vpython-jupyter

3D visualization made easy
MIT License
142 stars 64 forks source link

Fix TypeError resulting from lack of __rmul__ #233

Closed UZ9 closed 10 months ago

UZ9 commented 1 year ago

The current codebase has this comment regarding the lack of __rmul__ in cyvector.pyx:

    def __mul__(self, other):  ## in cython order of arguments is arbitrary, rmul doesn't exist
        if isinstance(other, (int, float)):
            return vector(self._x * other, self._y * other, self._z * other)
        elif isinstance(self, (int, float)):
            return vector(self * other._x, self * other._y, self * other._z)
        return NotImplemented

However, when cython is not installed the following will throw a TypeError due to a lack of __rmul:

1.0 * vector(3, 4, 8)

This contradicts the following statement in the README:

If you don't have a compilier vpython should still work, but code that generates a lot of vectors may run a little slower.

The commit attached to this PR ensures the __rmul__ seen in vector.py is also present in cypython.pyx.

Relevant discussion on forum: https://groups.google.com/g/vpython-users/c/6ojA7FxcILA

rubdos commented 1 year ago

(From your comment on the forum)

encounters the TypeError. Seems like there was no rmul for the vector class, with the comment "in cython order of arguments is arbitrary, rmul doesn't exist."

This seems strange considering Cython 3.0.2 is already installed, but something likely worth mentioning in the documentation.

I have Cpython 3.0.5 installed, and I would expect this to "just work" (and be faster) without your patch. Did you find out why having Cython 3.0.x around doesn't help this issue?

mwcraig commented 10 months ago

Closing/reopening to run the the tests on Python 3.12

mwcraig commented 10 months ago

Thanks, @UZ9, merging!