-
Notifications
You must be signed in to change notification settings - Fork 655
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-#7381: Fix Series binary operators ignoring fill_value #7394
FIX-#7381: Fix Series binary operators ignoring fill_value #7394
Conversation
Signed-off-by: Jonathan Shi <[email protected]>
|
||
|
||
@pytest.mark.parametrize("op", ["eq", "ge", "gt", "le", "lt", "ne"]) | ||
def test_logical_binary_with_list(op): |
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.
How does this relate to the problem with fill_value
?
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.
My first pass at implementing fill_value
created errors for these functions when the second operand was not a Series, so I added this test to make sure it didn't break.
modin/pandas/series.py
Outdated
level=level, | ||
fill_value=fill_value, | ||
axis=axis, | ||
squeeze_other=isinstance(other, (pandas.Series, Series)), |
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.
Why do we need to do squeeze
in case of a series?
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'm using the Binary.register
/BinaryDefault.register
operators in the query compiler to implement these functions, which treats the two arguments as pandas.DataFrame
s when applying the passed function to partitions. I'm not sure if there's a more intelligent way of forcing the arguments to be treated as pandas.Series instead.
Co-authored-by: Anatoly Myachev <[email protected]>
What do these changes do?
Many binary operators in the frontend Series class were not passing their
fillna
parameters to the query compiler (not sure why this wasn't caught in linting). For arithmetic functions, the fix is simply to propagatefillna
, but for logical operators (eq
/ge
etc.), the equivalent DataFrame method does not take afillna
parameter, so we need to add a separate query compiler method to handle this case.flake8 modin/ asv_bench/benchmarks scripts/doc_checker.py
black --check modin/ asv_bench/benchmarks scripts/doc_checker.py
git commit -s
docs/development/architecture.rst
is up-to-date