-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
ENH: add rmod methods to frame and series #4035
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
Conversation
your sure this is necessary #3590 fixed these mod issues |
as it stands mod follows |
this pr fixes that |
so the broadcasting is wrong? |
@@ -874,7 +876,8 @@ def __contains__(self, key): | |||
default_axis=None, fill_zeros=np.inf) | |||
__rpow__ = _arith_method(lambda x, y: y ** x, '__rpow__', | |||
default_axis=None) | |||
__rmod__ = _arith_method(operator.mod, '__rmod__', default_axis=None, fill_zeros=np.nan) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jreback this line here was calling mod(x, y) when it should be mod(y, x) that's why it was appearing commutative. rmod == mod
here when it shouldn't
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
incidentally this was caught by my eval
test suite
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh ok that's fine then
as long as exiting and new tests pass
passes....i'll wait a bit to merge to make sure i didn't forget anything |
I tried your fix, didn't realize this was an issue...its essentially not broadcasting..so good to go |
ENH: add rmod methods to frame and series
No description provided.