Repository: pandas-dev/pandas
Pull request: BUG: pd.to_datetime() throws if caching is on with Null-like arguments
- [x] closes #22305
- [x] tests added / passed
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] whatsnew entry

The problem was in contradictory behavior between `pd.Index([pd.NaT, None]).is_unique` that returns `False` and `algorithms.unique([pd.NaT, None])` that returns `array([NaT, None], dtype=object)` equivalent to the object being unique.

We are only using `Index(arg).unique()` now.

The test written by @realead was added to verify changes.

Problem statement:
pd.to_datetime() throws if caching is on with Null-like arguments
#### Code Sample, a copy-pastable example if possible

```python
import pandas as pd
result = pd.to_datetime([pd.NaT, None], cache=True)
```
#### Problem description

It results in error:

> ...
> ~/anaconda3/lib/python3.6/site-packages/pandas/core/indexes/base.py in get_indexer(self, target, method, limit, tolerance)
>    3242 
>    3243         if not self.is_unique:
> -> 3244             raise InvalidIndexError('Reindexing only valid with uniquely'
>    3245                                     ' valued Index objects')
>    3246 
> 
> InvalidIndexError: Reindexing only valid with uniquely valued Index objects
> 

#### Expected Output

The same as `result = pd.to_datetime([pd.NaT, None],cache=False)`:

`DatetimeIndex(['NaT', 'NaT'], dtype='datetime64[ns]', freq=None)`

#### Output of ``pd.show_versions()``

<details>
INSTALLED VERSIONS
------------------
commit: None
python: 3.6.2.final.0
python-bits: 64
OS: Linux
OS-release: 4.4.0-53-generic
machine: x86_64
processor: x86_64
byteorder: little
LC_ALL: None
LANG: en_US.UTF-8
LOCALE: en_US.UTF-8

pandas: 0.23.4
pytest: 3.2.1
pip: 10.0.1
setuptools: 36.5.0.post20170921
Cython: 0.28.3
numpy: 1.13.1
scipy: 1.1.0
pyarrow: None
xarray: None
IPython: 6.1.0
sphinx: 1.6.3
patsy: 0.4.1
dateutil: 2.6.1
pytz: 2017.2
blosc: None
bottleneck: 1.2.1
tables: 3.4.2
numexpr: 2.6.2
feather: None
matplotlib: 2.0.2
openpyxl: 2.4.8
xlrd: 1.1.0
xlwt: 1.3.0
xlsxwriter: 0.9.8
lxml: 3.8.0
bs4: 4.6.0
html5lib: 0.9999999
sqlalchemy: 1.1.13
pymysql: None
psycopg2: None
jinja2: 2.9.6
s3fs: 0.1.3
fastparquet: None
pandas_gbq: None
pandas_datareader: None
[paste the output of ``pd.show_versions()`` here below this line]

</details>

Hints from issue discussion:
Thanks for the report! Looking into it / a fix is certainly welcome.
We have this test:

https://github.com/pandas-dev/pandas/blob/eb0ac5437f9fee2939b3c3c4dab3ed745b5a9c38/pandas/tests/indexes/datetimes/test_tools.py#L505

Where currently this test with `cache=True` is skipped because of #18111 which I think is similar to what you found.
In the end it comes down to the following difference between `unique` and `infer_dtype`:

While unique [mangles `pd.NaT` and `np.nan` but not `None`](https://github.com/pandas-dev/pandas/blob/master/pandas/_libs/hashtable_class_helper.pxi.in#L852):

```
            # `val is None` below is exception to prevent mangling of None and
            # other NA values; note however that other NA values (ex: pd.NaT
            # and np.nan) will still get mangled, so many not be a permanent
            # solution; see GH 20866
           if not checknull(val) or val is None:
                  ...
```

[`infer_dtype` magles them all](https://github.com/pandas-dev/pandas/blob/73909638515327412997e2037750fa9b57e3f7c0/pandas/_libs/lib.pyx#L1160):

```
        if val is None or util.is_nan(val):
            pass
        elif val is NaT:
            seen_pdnat = True
        else:
            ....
```

I ask myself, whether this `infer_dtype`-business is really necessary and maybe can be avoided.

This issue kind of blocks PR #22296, because PR #22296  fixes the mangling of `pd.NaT` and `np.nan` in `unique`, and thus adding another problem similar to `[None, pd.NaT]`: i.e. `[np.nan, pd.NaT]` - and this case is covered in the test suite. 
This test should probably pass once it is fixed:

```
    def test_NA_values_with_cache(self):
        # GH 22305
        na_values = [None, np.nan, pd.NaT]
        # check pairwise, that no pair of na values
        # is mangled
        for f in na_values:
            for s in na_values:
                if f is not s:  # otherwise not unique
                    expected = Index([NaT, NaT], dtype='datetime64[ns]')
                    result = to_datetime([f, s], cache=True)
                    tm.assert_index_equal(result, expected)
```

Commit message: added whatsnew entry

Patch to review:
diff --git a/doc/source/whatsnew/v0.25.0.rst b/doc/source/whatsnew/v0.25.0.rst
index c441244b4415d..f88c2923686cf 100644
--- a/doc/source/whatsnew/v0.25.0.rst
+++ b/doc/source/whatsnew/v0.25.0.rst
@@ -266,7 +266,7 @@ Datetimelike
 ^^^^^^^^^^^^
 
 - Bug in :func:`to_datetime` which would raise an (incorrect) ``ValueError`` when called with a date far into the future and the ``format`` argument specified instead of raising ``OutOfBoundsDatetime`` (:issue:`23830`)
--
+- Bug in :func:`to_datetime` which would raise ``InvalidIndexError`` when called with ``cache=True``, with ``arg`` including several different "not a numbers" (:issue:`22305`)
 -
 -
 
diff --git a/pandas/core/tools/datetimes.py b/pandas/core/tools/datetimes.py
index 66d563a7c6f85..506ac00946109 100644
--- a/pandas/core/tools/datetimes.py
+++ b/pandas/core/tools/datetimes.py
@@ -50,11 +50,12 @@ def _maybe_cache(arg, format, cache, convert_listlike):
     from pandas import Series
     cache_array = Series()
     if cache:
-        # Perform a quicker unique check
         from pandas import Index
-        if not Index(arg).is_unique:
-            unique_dates = algorithms.unique(arg)
-            cache_dates = convert_listlike(unique_dates, True, format)
+        # GH26078
+        unique_dates = Index(arg).unique()
+        if len(unique_dates) < len(arg):
+            cache_dates = convert_listlike(unique_dates.to_numpy(),
+                                           True, format)
             cache_array = Series(cache_dates, index=unique_dates)
     return cache_array
 
diff --git a/pandas/tests/indexes/datetimes/test_tools.py b/pandas/tests/indexes/datetimes/test_tools.py
index eaf689cfa1c21..a592ef941484e 100644
--- a/pandas/tests/indexes/datetimes/test_tools.py
+++ b/pandas/tests/indexes/datetimes/test_tools.py
@@ -1630,6 +1630,15 @@ def test_parsers(self, date_str, expected, cache):
                                  yearfirst=yearfirst)
             assert result7 == expected
 
+    @pytest.mark.parametrize('cache', [True, False])
+    def test_na_values_with_cache(self, cache, unique_nulls_fixture,
+                                  unique_nulls_fixture2):
+        # GH22305
+        expected = Index([NaT, NaT], dtype='datetime64[ns]')
+        result = to_datetime([unique_nulls_fixture, unique_nulls_fixture2],
+                             cache=cache)
+        tm.assert_index_equal(result, expected)
+
     def test_parsers_nat(self):
         # Test that each of several string-accepting methods return pd.NaT
         result1, _, _ = parsing.parse_time_string('NaT')


Code review comments:
diff_hunk: @@ -266,7 +266,7 @@ Datetimelike
 ^^^^^^^^^^^^
 
 - Bug in :func:`to_datetime` which would raise an (incorrect) ``ValueError`` when called with a date far into the future and the ``format`` argument specified instead of raising ``OutOfBoundsDatetime`` (:issue:`23830`)
--
+- Bug in :func:`to_datetime` which would raise ``InvalidIndexError`` when called with ``cache=True``, with ``arg`` including several different "not a numbers" (:issue:`22305`)
path: doc/source/whatsnew/v0.25.0.rst
text: @user1:
show more text of the error: InvalidIndexError: Reindexing only valid with uniquely valued Index objects

this is also confusing what you wrote: ``including several different "not a numbers"``

pls re-word