-
-
Notifications
You must be signed in to change notification settings - Fork 19k
BUG: Fix incorrect FutureWarning for logical ops on pyarrow bool Series (#62260) #62290
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
BUG: Fix incorrect FutureWarning for logical ops on pyarrow bool Series (#62260) #62290
Conversation
...e for logical operations
the dosctring check required me to return a true or false bool only
...manual pre-commit hooks (pull_request)Failing after 16m
...or truthy values like 1
Kindly review my PR for improving the logical operation of arrays and aligning them with Kleene's Principles. Please tell me if any issues. Finally heading out to touch some grass :)
@simonjayhawkins Kindly review my PR and let me know 👍 (If in testing phase or not)
This Wikipedia article may be useful to understand the changes in this PR: https://en.wikipedia.org/wiki/Three-valued_logic#Kleene_and_Priest_logics
It's also important to point-out that this is a breaking change and should have an entry in doc/source/whatsnew/v3.0.0.rst
.
Also, bool[pyarrow]
already implements 3VL
import pandas as pd index = list("FUT") a = pd.Series([False, None, True], index=index, dtype="bool[pyarrow]") t = pd.Series([True] * 3, index=index, dtype = "bool[pyarrow]") u = pd.Series([None] * 3, index=index, dtype = "bool[pyarrow]") f = pd.Series([False] * 3, index=index, dtype = "bool[pyarrow]") print("negation") print(~a) methods = ["__and__", "__or__", "__xor__"] for method in methods: print(method) fn = getattr(a, method) observed = pd.DataFrame(dict(F=fn(f), U=fn(u), T=fn(t)), index=index) print(observed)
Output
negation
F True
U <NA>
T False
dtype: bool[pyarrow]
__and__
F U T
F False False False
U False <NA> <NA>
T False <NA> True
__or__
F U T
F False <NA> True
U <NA> <NA> True
T True True True
__xor__
F U T
F False <NA> True
U <NA> <NA> <NA>
T True <NA> False
@Alvaro-Kothe Yes I agree, the pyarrow implementation follows the kleene principle whereas bool does not. Thank you for attaching the wiki article 👍 The core members of pandas lib are the ones who fill the whatsnew right?
@Tarun2605 Usually, whoever creates the pull request should fill the whatsnew
@Alvaro-Kothe Ohhhh!! Could you please tell me where do I fill it out? Thank you so much btw man
Utilize your own discretion or await guidance from a core member.
Okkk
@simonjayhawkins Kindly review my PR or have another member review it please
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.
Thanks @Tarun2605 for the PR.
This PR is changing tested behavior.
#62260 (comment) states
For the result on main / pandas 3.0, this actually seems correct to me.
so i'm not expecting to see any changed behavior other than w.r.t the warning
FutureWarning: Operation between non boolean Series with different indexes will no longer return a boolean result in a future version. Cast both Series to object type to maintain the prior behavior.
Thanks @Tarun2605 for the PR.
This PR is changing tested behavior.
#62260 (comment) states
For the result on main / pandas 3.0, this actually seems correct to me.
so i'm not expecting to see any changed behavior other than w.r.t the warning
FutureWarning: Operation between non boolean Series with different indexes will no longer return a boolean result in a future version. Cast both Series to object type to maintain the prior behavior.
Thank you sob much for your reply.
Yes, I was initially set out to change that only but then i noticed how kleene's principle was not being followed bool arrays. So should we let this inconsistency go with null values?
Also,
bool[pyarrow]
already implements 3VLimport pandas as pd index = list("FUT") a = pd.Series([False, None, True], index=index, dtype="bool[pyarrow]") t = pd.Series([True] * 3, index=index, dtype = "bool[pyarrow]") u = pd.Series([None] * 3, index=index, dtype = "bool[pyarrow]") f = pd.Series([False] * 3, index=index, dtype = "bool[pyarrow]") print("negation") print(~a) methods = ["__and__", "__or__", "__xor__"] for method in methods: print(method) fn = getattr(a, method) observed = pd.DataFrame(dict(F=fn(f), U=fn(u), T=fn(t)), index=index) print(observed)Output
negation F True U <NA> T False dtype: bool[pyarrow] __and__ F U T F False False False U False <NA> <NA> T False <NA> True __or__ F U T F False <NA> True U <NA> <NA> True T True True True __xor__ F U T F False <NA> True U <NA> <NA> <NA> T True <NA> False
As nicely stated these examples will break or give wrong values when the array d type is converted to bool. Thats the inconsistency i was trying to solve.
Thank you
Thats the inconsistency i was trying to solve.
If this PR is not addressing the FutureWarning then please remove the link to that issue.
Can you open an specific issue to discuss this proposal or link to an existing issue rather than us discussing on a PR. PRs can get closed or go stale and the discussion then has less visibility.
Thats the inconsistency i was trying to solve.
If this PR is not addressing the FutureWarning then please remove the link to that issue.
Can you open an specific issue to discuss this proposal or link to an existing issue rather than us discussing on a PR. PRs can get closed or go stale and the discussion then has less visibility.
I see, ok sure I will open an issue and have discussion and then raise my PR
Uh oh!
There was an error while loading. Please reload this page.
This pull request introduces support for Kleene's three-valued logic (handling True, False, and NA) in pandas logical operations on arrays containing missing values. The main changes include new helper functions to safely evaluate boolean logic with missing values, modifications to the logical operation implementation, and updates to tests to reflect the new behavior.
Enhancements to logical operations with missing values:
is_nullable_bool
,safe_is_true
, andalignOutputWithKleene
helper functions inpandas/core/ops/array_ops.py
to enable elementwise logical operations that correctly handle NA values using Kleene logic.logical_op
inpandas/core/ops/array_ops.py
to use Kleene logic when both operands are boolean arrays (possibly with NA), ensuring correct propagation of unknowns.Test updates for new logic:
TestDataFrameLogicalOperators
inpandas/tests/frame/test_logical_ops.py
to match the new Kleene logic semantics, where logical operations with NA now return NA or propagate True/False according to Kleene's rules.test_logical_with_nas
test to expect results consistent with Kleene logic, ensuring that logical operations involving NA and True/False yield the correct outcomes.- [x] closes BUG: Incorrect Future warning using a logical operation between two pyarrow boolean series #62260pre-commit run --all-files
What does this PR change?
This pull request updates the Arrow extension array implementation in pandas to improve how missing values are handled during logical operations on
bool
.Previously, operations like
&
or|
between two bool series was not following Kleene's Logic.How was this fixed?