-
-
Notifications
You must be signed in to change notification settings - Fork 18.5k
CLN: dont special-case DatetimeArray indexing #36654
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
Conversation
can you add some tests that lock this down (eg from the OP). I think this would be ok to push for 1.2 with a whatsnew note. |
whatsnew+test+green |
any other thoughts here? after this i can get back to de-duplicating EA code |
def test_getitem_boolean_dt64_copies(): | ||
# GH#36210 | ||
dti = date_range("2016-01-01", periods=4, tz="US/Pacific") | ||
key = np.array([True, True, False, False]) |
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 replicate the OP here as well (e.g. add the slice test for this too)
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.
not sure i understand. this is exactly the example from the 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.
the OP had 2 example inputs
ser1 = pd.Series(dti._data)
ser2 = pd.Series(range(4))
Test amended + green |
Thanks Brock! |
* CLN: dont special-case DatetimeArray indexing * use parent class _validate_getitem_key * test, whatsnew * update test
* CLN: dont special-case DatetimeArray indexing * use parent class _validate_getitem_key * test, whatsnew * update test
black pandas
git diff upstream/master -u -- "*.py" | flake8 --diff
cc @jorisvandenbossche