Skip to content
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: #35830 Restore index & slice by date to DatetimeIndex #39380

Closed
wants to merge 3 commits into from

Conversation

davidia
Copy link

@davidia davidia commented Jan 24, 2021

#35830 DatetimeIndex has supported indexing by date for many many years. This was broken after 1.0.5. This attempts to restore support by converting a date to Timestamp in get_loc and _validate_searchsorted_value. This restores indexing slicing, and asof but in does not work. It seems the best solution would be to do the conversion in DatetimeLikeArrayMixin._validate_scalar but this breaks a test looking for an expected FutureWarning in TestDataFrameIndexing::test_loc_setitem_datetime_coercion warning a Timestamp can't be compared to a date

DatetimeIndex has supported indexing by date for many many years. This was broken
after 1.0.5. This attempts to restore support by promoting date a non--tz aware
datetime at midnight.
@davidia
Copy link
Author

davidia commented Jan 24, 2021

I can add more tests for slicing and asof (they work) if this will be considered for merging

@davidia davidia changed the title BUG: #35830 Restore index by Date to DatetimeIndex BUG: #35830 Restore index & slice by date to DatetimeIndex Jan 24, 2021
Copy link
Contributor

@jreback jreback left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i would be ok with a warning but we deliberately change this as it's incorrect

@@ -552,6 +552,11 @@ def _validate_scalar(
-------
self._scalar_type or NaT
"""

if type(value) == date:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this (and the similar test below) be isinstance(value, date)?

@github-actions
Copy link
Contributor

This pull request is stale because it has been open for thirty days with no activity. Please update or respond to this comment if you're still interested in working on this.

@github-actions github-actions bot added the Stale label Feb 25, 2021
@mroeschke
Copy link
Member

Closing as it appears this PR has gotten stale. Let us know if you'd like to continue adding a deprecation warning for this behavior and we'd be happy to reopen.

@mroeschke mroeschke closed this Apr 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

API: indexing DatetimeIndex with date object
4 participants