-
- Notifications
You must be signed in to change notification settings - Fork 19.4k
ENH: Implement Kleene logic for BooleanArray #29842
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
Changes from 1 commit
bb904cb 13c7ea3 fff786f 4067e7f 708c553 c56894e 2e9d547 373aaab 7f78a64 36b171b 747e046 d0a8cca fe061b0 9f9e44c 0a34257 2ba0034 2d1129a a24fc22 77dd1fc 7b9002c c18046b 1237caa 2ecf9b8 87aeb09 969b6dc 1c9ba49 8eec954 cb47b6a 2a946b9 efb6f8b 004238e 5a2c81c 7032318 bbb7f9b ce763b4 5bc5328 457bd08 31c2bc6 File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
- Loading branch information
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| | @@ -356,6 +356,13 @@ def test_ufunc_reduce_raises(values): | |
| | ||
| | ||
| class TestLogicalOps(BaseOpsUtil): | ||
| def test_numpy_scalars_ok(self, all_logical_operators): | ||
| a = pd.array([True, False, None], dtype="boolean") | ||
| op = getattr(a, all_logical_operators) | ||
| | ||
| tm.assert_extension_array_equal(op(True), op(np.bool(True))) | ||
| tm.assert_extension_array_equal(op(False), op(np.bool(False))) | ||
| | ||
| def get_op_from_name(self, op_name): | ||
| short_opname = op_name.strip("_") | ||
| short_opname = short_opname if "xor" in short_opname else short_opname + "_" | ||
| | @@ -403,6 +410,12 @@ def test_logical_nan_raises(self, all_logical_operators): | |
| with pytest.raises(ValueError, match=msg): | ||
| getattr(a, op_name)(np.nan) | ||
| | ||
| @pytest.mark.parametrize("other", ["a", 1]) | ||
| def test_non_bool_or_na_other_raises(self, other, all_logical_operators): | ||
| a = pd.array([True, False], dtype="boolean") | ||
| with pytest.raises(TypeError, match=str(type(other).__name__)): | ||
| getattr(a, all_logical_operators)(other) | ||
| | ||
| def test_kleene_or(self): | ||
| Contributor Author There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A careful review of these new test cases would be greatly appreciated. I've tried to make them as clear as possible, while covering all the cases. Member There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I went through the tests, very clear, added a few comments, for the rest looks good to me! | ||
| # A clear test of behavior. | ||
| a = pd.array([True] * 3 + [False] * 3 + [None] * 3, dtype="boolean") | ||
| | ||
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.
You'll need to add
np.bool_as well I think (and tests for this), as a numpy scalar is not an instance of a bool: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.
Also,
other is pd.NA or isinstance(other, bool)is a bit faster, although I am not sure we care about the 60 ns differenceThere 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.
Hmm, but I see that you actually already have a test for numpy scalars?
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.
Ah, I was tripped up by
np.boolvsnp.bool_.