-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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(python): validate operator arithmetic with None
, fix Series
edge-case
#13780
fix(python): validate operator arithmetic with None
, fix Series
edge-case
#13780
Conversation
86c4bba
to
724487d
Compare
null
, fix Series
edge-caseNone
, fix Series
edge-case
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.
If we want to support multiplying by None
leading to a full None result, we should implement this on the Rust side and support multiplication and other operations for Null
series.
I think this was 'broken' in the update where empty Series default to the Null
type.
EDIT: I didn't read the description properly... maybe this is an OK fix.
That worked before, and still works now ;)
Indeed; operations against |
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.
Apologies, didn't give this the correct attention the first time around (it was late). This is a good fix!
I have one comment then this can be merged.
Co-authored-by: Stijn de Gooijer <stijn@degooijer.io>
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.
Good to merge if CI is green 👍
…dge-case (pola-rs#13780) Co-authored-by: Stijn de Gooijer <stijn@degooijer.io>
…dge-case (pola-rs#13780) Co-authored-by: Stijn de Gooijer <stijn@degooijer.io>
Adds test coverage for operator arithmetic against
None
(and tests the associated named methods). Discovered thatSeries
was inconsistently raising an error where the same ops onExpr
andDataFrame
succeeded.Note:
This may be an implicit regression of some kind (from the
0.19.x
series) as it was found via unit testing while trying to upgrade our work codebase to0.20.x
🤔Example
All of the following formulations work consistently...
... only this variant failed:
I've fixed this and added the missing test coverage.