DatetimeIndex get_loc validates date object
State: closed

- [ ] closes #35466
- [ ] tests added / passed
- [ ] passes `black pandas`
- [ ] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry

Adding the date object try cast in get_loc.

Labels:
name: Datetime
description: Datetime data dtype
name: Needs Discussion
description: Requires discussion from core team before further action
name: Index
description: Related to the Index class or subclasses

Comments:
Hello @knabben! Thanks for updating this PR. We checked the lines you've touched for [PEP 8](https://www.python.org/dev/peps/pep-0008) issues, and found:











There are currently no PEP 8 issues detected in this Pull Request. Cheers! :beers: 

##### Comment last updated at 2020-11-04 15:43:34 UTC
Do we want `dti == date` to do partial-matching the same way we would with a date-only string?  (i guess Id be OK punting on this)(
> Do we want `dti == date` to do partial-matching the same way we would with a date-only string? (i guess Id be OK punting on this)(

@jbrockmendel feel free to catch this, looks like you have been working with the flow on GH#19800 and GH#19301.

But having dti == date, looks ok, since the date is timestamped after. 
@knabben can you add a release note to doc\source\whatsnew\v1.1.1.rst
It's not clear to me that the behavior on master is incorrect. Shouldn't we be following the behavior of Python's `dict`, which doesn't consider dates to be datetimes at midnight?

```python
In [18]: d = {datetime.datetime(2000, 1, 1): 1}

In [19]: datetime.date(2000, 1, 1) in d
Out[19]: False
```
> It's not clear to me that the behavior on master is incorrect. Shouldn't we be following the behavior of Python's `dict`, which doesn't consider dates to be datetimes at midnight?
> 
> ```python
> In [18]: d = {datetime.datetime(2000, 1, 1): 1}
> 
> In [19]: datetime.date(2000, 1, 1) in d
> Out[19]: False
> ```

I had the same question in the issue, maybe a date in DateTimeIndex should never be considered and this now should be the correct behavior, this is clear after adding, *date* in the recognized_scalars, started to break a lot of tests, that were not expecting this to be there (like blocks, internals, etc):

```_recognized_scalars = (datetime, np.datetime64, date)```

It was passing before because [get_loc](https://github.com/pandas-dev/pandas/blob/b687cd4d9e520666a956a60849568a98dd00c672/pandas/core/indexes/datetimes.py#L723), was using a fallback for this translation.

cc @jbrockmendel 
I think get_loc needs to match __eq__, which needs to match
Timestamp.__eq__.  I think ATM Timestamp doesn’t match stdlib datetime
behavior, which id be OK with changing (following deprecation cycle)

On Fri, Jul 31, 2020 at 2:37 PM Amim Knabben <notifications@github.com>
wrote:

> It's not clear to me that the behavior on master is incorrect. Shouldn't
> we be following the behavior of Python's dict, which doesn't consider
> dates to be datetimes at midnight?
>
> In [18]: d = {datetime.datetime(2000, 1, 1): 1}
> In [19]: datetime.date(2000, 1, 1) in dOut[19]: False
>
> I had the same question in the issue, maybe a date in DateTimeIndex should
> never be considered and this now should be the correct behavior, this is
> clear that after adding, *date*:
>
> _recognized_scalars = (datetime, np.datetime64, date)
>
> Break a lot of tests, that were not expecting this to be there (like
> blocks, internals, etc), it was passing because get_loc, was using a
> fallback for this translation.
>
> cc @jbrockmendel <https://github.com/jbrockmendel>
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/pandas-dev/pandas/pull/35478#issuecomment-667385200>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AB5UM6HQJM6Y2HNWY4JTKJDR6M2QLANCNFSM4PN7Q52Q>
> .
>

closing the PR for the lack of time to investigate it further, for those going on:

1. It should be keep the behavior of date in DatetimeIndex equivalence, at least while in the deprecation cycle.
2. For the long run assess if this behavior still makes sense, deprecating otherwise.
> > It's not clear to me that the behavior on master is incorrect. Shouldn't we be following the behavior of Python's dict, which doesn't consider dates to be datetimes at midnight?
> 
> I think get_loc needs to match `__eq__`, which needs to match `Timestamp.__eq__`.  

Indeed, right now, Timestamp considers a `date` to be equal:

```
In [33]: pd.Timestamp("2012-01-01") == datetime.date(2012, 1, 1)  
Out[33]: True
```

So I think if we want to change this to False, that should go through a deprecation cycle, and the same for the indexing behaviour.
Although that for Index/Array, we return False ...

```
In [34]: pd.array([pd.Timestamp("2012-01-01")]) == datetime.date(2012, 1, 1) 
Out[34]: array([False])

In [35]: pd.Index([pd.Timestamp("2012-01-01")]) == datetime.date(2012, 1, 1) 
Out[35]: array([False])
```
Just tested this PR to see if it would fix the original issue, it looks like it still doesn't :(
```py
>>> datetime.date(2000,1,1) in pd.DatetimeIndex(['2000-01-01'])
False
```
> Indeed, right now, Timestamp considers a date to be equal:

It also looks like Timestamp fails to raise TypeError when doing checking `Timestamp[aware] < date`.

Also looks like `Timestamp(not_midnight) == date` returns True because of NotImplemented semantics
@knabben can you merge master
-1, DTI.get_loc _should_ raise on a date object.  xref #36131
> -1, DTI.get_loc _should_ raise on a date object. xref #36131

good point

thanks for the effort @knabben love to have you contribute on another issue!