-
Notifications
You must be signed in to change notification settings - Fork 174
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: boolean and/or expressions with null #3544
Conversation
CodSpeed Performance ReportMerging #3544 will degrade performances by 10.05%Comparing Summary
Benchmarks breakdown
|
tests/series/test_comparisons.py
Outdated
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.
I only touched this file to update test values but Ruff wanted to reformat it as well
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3544 +/- ##
=======================================
Coverage 77.69% 77.69%
=======================================
Files 710 710
Lines 86896 86938 +42
=======================================
+ Hits 67511 67550 +39
- Misses 19385 19388 +3
|
), | ||
)) | ||
} | ||
|
||
match (self.len(), rhs.len()) { | ||
(x, y) if x == y => { |
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.
maybe better names than x
and y
?
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.
This naming seems to exist across all of the ops so I think I will just leave it be. Don't want to make this PR too big or stray away from convention
@andrewgazelka could you give this a second look? |
Resolves #3512
A fun truth table ($V_{x}$ is the validity $x$ , $L$ is left, and $R$ is right):