-
-
Notifications
You must be signed in to change notification settings - Fork 18.2k
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: Datetimelike equality comparisons with Categorical #38986
Conversation
ftrihardjo
commented
Jan 6, 2021
•
edited by jreback
Loading
edited by jreback
- closes BUG: Datetimelike equality comparisons with Categorical #30699
- tests added / passed
- Ensure all linting tests pass, see here for how to run them
- whatsnew entry
@@ -162,6 +162,20 @@ def _compare_other(self, s, data, op_name, other): | |||
# with (some) integers, depending on the value. | |||
pass | |||
|
|||
def test_compare_with_Categorical(self): |
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.
can you parameterize this over the types (date_range and such),
also pls add a case for !=
@@ -162,6 +162,20 @@ def _compare_other(self, s, data, op_name, other): | |||
# with (some) integers, depending on the value. | |||
pass | |||
|
|||
def test_compare_with_Categorical(self): |
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.
We shouldn't add additional tests to this file (some other files in this directory have a note about that, just opened a PR to add that to this file as well -> #39003)
@ftrihardjo can you move the test to eg /tests/arrays/test_datetimelike.py
? (not fully sure what is the best place to put those nowadays) There also already seems to be a test_compare_categorical_dtype
test in that file, so you can put it close to that one (or see to what extent it overlaps)
@@ -85,6 +85,24 @@ def test_compare_len1_raises(self): | |||
with pytest.raises(ValueError, match="Lengths must match"): | |||
idx <= idx[[0]] | |||
|
|||
def test_compare_with_Categorical(self): |
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.
can you parameterize over pd.date_range,timedetla_range and period_range
Hello @ftrihardjo! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found: There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻 Comment last updated at 2021-01-08 01:13:09 UTC |
thanks @ftrihardjo |