Repository: pandas-dev/pandas
Pull request: Fixes for #16896(TimedeltaIndex indexing regression for strings)
Fixed a regression for indexing with a string with a ``TimedeltaIndex`` (#16896)

I am not a fan of the catch all except, but there is no guarantee in `_get_dtype_type`. As part of this PR, we can ensure that all type checks return True/False. `_get_dtype_type` should probably return a type or a TypeError... Nothing else. Let me know if you want to do that as well.

 - [x] closes #16896
 - [x] tests added / passed
 - [x] passes ``git diff upstream/master --name-only -- '*.py' | flake8 --diff`` (On Windows, ``git diff upstream/master -u -- "*.py" | flake8 --diff`` might work as an alternative.)
 - [x] whatsnew entry


Problem statement:
BUG: TimedeltaIndex indexing regression for strings in v0.20.3
With pandas 0.20.2:
```
In [1]: import pandas as pd

In [3]: df = pd.DataFrame({'x': range(3)}, pd.to_timedelta(range(3), unit='days'))

In [4]: df.loc['0 days']
Out[4]:
x    0
Name: 0 days 00:00:00, dtype: int64

In [5]: df.index
Out[5]: TimedeltaIndex(['0 days', '1 days', '2 days'], dtype='timedelta64[ns]', freq=None)

In [7]: pd.__version__
Out[7]: '0.20.2'
```

Now, with 0.20.3:
```
In [1]: import pandas as pd

In [2]: df = pd.DataFrame({'x': range(3)}, pd.to_timedelta(range(3), unit='days'))

In [3]: df.loc['0 days']
---------------------------------------------------------------------------
KeyError                                  Traceback (most recent call last)
/Users/shoyer/conda/envs/xarray-dev/lib/python3.5/site-packages/pandas/core/indexing.py in _has_valid_type(self, key, axis)
   1433                 if not ax.contains(key):
-> 1434                     error()
   1435             except TypeError as e:

/Users/shoyer/conda/envs/xarray-dev/lib/python3.5/site-packages/pandas/core/indexing.py in error()
   1428                 raise KeyError("the label [%s] is not in the [%s]" %
-> 1429                                (key, self.obj._get_axis_name(axis)))
   1430

KeyError: 'the label [0 days] is not in the [index]'

During handling of the above exception, another exception occurred:

KeyError                                  Traceback (most recent call last)
<ipython-input-3-17caa55b0d41> in <module>()
----> 1 df.loc['0 days']

/Users/shoyer/conda/envs/xarray-dev/lib/python3.5/site-packages/pandas/core/indexing.py in __getitem__(self, key)
   1326         else:
   1327             key = com._apply_if_callable(key, self.obj)
-> 1328             return self._getitem_axis(key, axis=0)
   1329
   1330     def _is_scalar_access(self, key):

/Users/shoyer/conda/envs/xarray-dev/lib/python3.5/site-packages/pandas/core/indexing.py in _getitem_axis(self, key, axis)
   1549
   1550         # fall thru to straight lookup
-> 1551         self._has_valid_type(key, axis)
   1552         return self._get_label(key, axis=axis)
   1553

/Users/shoyer/conda/envs/xarray-dev/lib/python3.5/site-packages/pandas/core/indexing.py in _has_valid_type(self, key, axis)
   1440                 raise
   1441             except:
-> 1442                 error()
   1443
   1444         return True

/Users/shoyer/conda/envs/xarray-dev/lib/python3.5/site-packages/pandas/core/indexing.py in error()
   1427                                     "key")
   1428                 raise KeyError("the label [%s] is not in the [%s]" %
-> 1429                                (key, self.obj._get_axis_name(axis)))
   1430
   1431             try:

KeyError: 'the label [0 days] is not in the [index]'
```

This turned up in a failure for the xarray test suite.

@TomAugspurger can you guess the culprit PR offhand?

Hints from issue discussion:
#16638 seems most likely
cc @jdeschenes, mind taking a look?
After a bit of investigation:

```python
import pandas as pd

df = pd.DataFrame({'x': range(3)}, pd.to_timedelta(range(3), unit='days'))

df.loc['0 days 00:00:00']
```

This will work.

The culprit is the `is_timedelta64_dtype` function which was added to to the `get_loc` function as part of #16638.

```python
from pandas.core.dtypes.common import is_timedelta64_dtype
assert not is_timedelta64_dtype('0 days 00:00:00')

# But this one will fail with TypeError
assert not is_timedelta64_dtype('0 days')
```

The fix will be quite simple. Tweak the is_timedelta64_dtype so that it always return True or False. What do you think?


yes these should never raise (feel free to add a battery of tests)!

Commit message: Fixes for #16896

Fixed a regression for indexing with a string with a ``TimedeltaIndex``

Patch to review:
diff --git a/doc/source/whatsnew/v0.21.0.txt b/doc/source/whatsnew/v0.21.0.txt
index a5ee0e0ce2653..7b40e7ae04f2d 100644
--- a/doc/source/whatsnew/v0.21.0.txt
+++ b/doc/source/whatsnew/v0.21.0.txt
@@ -155,7 +155,7 @@ Indexing
 
 - When called with a null slice (e.g. ``df.iloc[:]``), the ``.iloc`` and ``.loc`` indexers return a shallow copy of the original object. Previously they returned the original object. (:issue:`13873`).
 - When called on an unsorted ``MultiIndex``, the ``loc`` indexer now will raise ``UnsortedIndexError`` only if proper slicing is used on non-sorted levels (:issue:`16734`).
-
+- Fixes regression in 0.20.3 that raises an index error when a string is used for ``TimedeltaIndex`` (:issue:`16896`).
 
 I/O
 ^^^
diff --git a/pandas/core/dtypes/common.py b/pandas/core/dtypes/common.py
index 2eebf3704253e..ac7189d108b0a 100644
--- a/pandas/core/dtypes/common.py
+++ b/pandas/core/dtypes/common.py
@@ -392,13 +392,15 @@ def is_timedelta64_dtype(arr_or_dtype):
     False
     >>> is_timedelta64_dtype(pd.Series([], dtype="timedelta64[ns]"))
     True
+    >>> is_timedelta64_dtype('0 days')
+    False
     """
 
     if arr_or_dtype is None:
         return False
     try:
         tipo = _get_dtype_type(arr_or_dtype)
-    except ValueError:
+    except:
         return False
     return issubclass(tipo, np.timedelta64)
 
diff --git a/pandas/tests/dtypes/test_common.py b/pandas/tests/dtypes/test_common.py
index c32e8590c5675..216238b5cd120 100644
--- a/pandas/tests/dtypes/test_common.py
+++ b/pandas/tests/dtypes/test_common.py
@@ -199,12 +199,15 @@ def test_is_datetime64tz_dtype():
 
 def test_is_timedelta64_dtype():
     assert not com.is_timedelta64_dtype(object)
+    assert not com.is_timedelta64_dtype(None)
     assert not com.is_timedelta64_dtype([1, 2, 3])
     assert not com.is_timedelta64_dtype(np.array([], dtype=np.datetime64))
     assert com.is_timedelta64_dtype(np.timedelta64)
     assert com.is_timedelta64_dtype(pd.Series([], dtype="timedelta64[ns]"))
-
+    assert not com.is_timedelta64_dtype('0 days')
     assert not com.is_timedelta64_dtype("0 days 00:00:00")
+    assert not com.is_timedelta64_dtype(["0 days 00:00:00"])
+    assert not com.is_timedelta64_dtype("NO DATE")
 
 
 def test_is_period_dtype():
diff --git a/pandas/tests/indexing/test_timedelta.py b/pandas/tests/indexing/test_timedelta.py
index be3ea8f0c371d..16af6b0084359 100644
--- a/pandas/tests/indexing/test_timedelta.py
+++ b/pandas/tests/indexing/test_timedelta.py
@@ -40,3 +40,9 @@ def test_list_like_indexing(self, indexer, expected):
                                 dtype="int64")
 
         tm.assert_frame_equal(expected, df)
+
+    def test_string_indexing(self):
+        df = pd.DataFrame({'x': range(3)}, index=pd.to_timedelta(range(3), unit='days'))
+        expected = df.iloc[0]
+        sliced = df.loc['0 days']
+        tm.assert_series_equal(sliced, expected)


Code review comments:
diff_hunk: @@ -155,7 +155,7 @@ Indexing
 
 - When called with a null slice (e.g. ``df.iloc[:]``), the ``.iloc`` and ``.loc`` indexers return a shallow copy of the original object. Previously they returned the original object. (:issue:`13873`).
 - When called on an unsorted ``MultiIndex``, the ``loc`` indexer now will raise ``UnsortedIndexError`` only if proper slicing is used on non-sorted levels (:issue:`16734`).
-
+- Fixes regression in 0.20.3 that raises an index error when a string is used for ``TimedeltaIndex`` (:issue:`16896`).
path: doc/source/whatsnew/v0.21.0.txt
text: @user1:
can you be a bit more explicty (e.g. say indexing with a string  when you have a TimedeltaIndex)