Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix TypeError resulting from lack of __rmul__ #233

Merged
merged 1 commit into from Feb 1, 2024
Merged

Conversation

UZ9
Copy link
Contributor

@UZ9 UZ9 commented Sep 26, 2023

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

@UZ9 UZ9 changed the title 🐛 Fix TypeError resulting from lack of __rmul__ Fix TypeError resulting from lack of __rmul__ Sep 26, 2023
@rubdos
Copy link

rubdos commented Nov 29, 2023

(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
Copy link
Contributor

mwcraig commented Feb 1, 2024

Closing/reopening to run the the tests on Python 3.12

@mwcraig mwcraig closed this Feb 1, 2024
@mwcraig mwcraig reopened this Feb 1, 2024
@mwcraig
Copy link
Contributor

mwcraig commented Feb 1, 2024

Thanks, @UZ9, merging!

@mwcraig mwcraig merged commit b44074c into vpython:master Feb 1, 2024
27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants