Repository: pandas-dev/pandas
Pull request: Let _get_dtype accept Categoricals and CategoricalIndex 
 - [ x] closes #16817
 - [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.)
 - [ ] whatsnew entry

Makes ``_get_dtype`` accept ``Categorical``s and ``CategoricalIndex``. Also passes relevant tests in ``test_common.py``.

See also discussion in #16845. Is there a manual for checking performance (can't imagine this affects performance, though)?

Problem statement:
BUG: _get_dtype accepts CategoricalDtype but throws TypeError with a Categorical
#### Problem description

I find it inconistent that ``_get_dtype`` accept a ``CategoricalDtype``, but will thow TypeError on ``Categorical``s. 

#### Code Sample, a copy-pastable example if possible

```python
>>> cats = pd.Categorical([1,2,3])
>>> pd.core.dtypes.common._get_dtype(cats.dtype)
category
>>> pd.core.dtypes.common._get_dtype(cats)
TypeError: data type not understood
```

#### Expected Output

```python
>>> cats = pd.Categorical([1,2,3])
>>> pd.core.dtypes.common._get_dtype(cats.dtype)
category
>>> pd.core.dtypes.common._get_dtype(cats)
category
```

#### Proposed solution

A solution could be to check for extension type (maybe just categorical dtype, not sure) + that ``arr_or_dtype`` has a dtype attribute:

```python
def _get_dtypes(arr_or_dtype):
   ... 
    elif isinstance(arr_or_dtype, IntervalDtype):
        return arr_or_dtype   # old code
   elif is_extension_type(arr_or_dtype) and hasattr(arr_or_dtype, 'dtype'):   # new code
        return arr_or_dtype.dtype   # new code
    elif isinstance(arr_or_dtype, string_types):   # old code
      ...
```

Hints from issue discussion:
This is an internal method, do you actually need it for something?
- We have *no* tests for these routines (either ``_get_dtype`` or ``_get_dtype_type``). rather we have bunches of integration tests via all of the ``is_*`` routines. Would be nice to nail down what this actually takes / returns.
- We should actually make ``_get_dtype_type`` just *call* ``_get_dtype`` (and have logic live there)
- We should be much more consistent about calling ``_get_dtype_type`` rather than the more specific ``_get_dtype``, IOW, all of the ``is_*`` routines *should* call ``_get_dtype_type`` (most of them do, but *some* just call ``_get_dtype``).

So if we can nail down these issues, then can address what else / what should ``_get_dtype`` actually take. I think It should actually be quite a bit more strict. We are a bit loose in the calling code for this. Maybe just need to add some assertions. These are core routines which are called implicity a lot.
@topper-123 love to have some help here with this :>
Hi,

I think that could be relatively easy, and could give it a shot. Some issues, though:

* I can't get ``pandas.test()`` to run without breaking. Likewise, I've tried ``py.test pandas/tests/dtypes/test_common.py`` after downloading the source code from github, but it fails. That makes me feel a bit apprehensive.
* I don't know C or Cython and can't prioritoze learning ATM, are those required?
* Some issue in ``_get_dtype`` are strange and I'm not sure the function always returns a correct result. E.g. 
  ```python
   >>> pd.core.dtypes.common._get_dtype(pd.Categorical([1,2,3]))
   TypeError: data type not understood
   >>> pd.core.dtypes.common._get_dtype(pd.Categorical([1,2,3]).dtype)
   category
   ```
   That is, the first one fail, while the second one succeeds, which IMO doesn't make sense (but maybe there is an aspect that I don't understand?). And because I can't get the tests to run, I can't poke around the issue so easily.

In short, if I can get some help on how to get the tests to run cleanly, and C/Cython knowledge isn't required, I'll give this a go. I'm on Windows 10 and use the Anaconda distribution.

have a look here: http://pandas.pydata.org/pandas-docs/stable/contributing.html

pretty complete instructions on how to run a dev environment on windows. its pretty straightforward, but if you need help pls ping.
And the fact that ``_get_dtype`` fails on categoricals, isn't that a bug (or a insufficient implementation)? It seems unfortunate that calling ``_get_dtype`` with a Pandas categorical should fail.

That is, should it be marked in the tests as ok or a failure that _get_dtype raises when given a categorical? 
its reasonable to accept a ``Categorical`` instance I think.
We want the consitency for ndarrays, e.g. we can accept an ndarray, or its dtype. Scalars raise.

```
In [8]: from pandas.core.dtypes.common import _get_dtype

In [9]: _get_dtype(1)
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-9-d4be99dce80c> in <module>()
----> 1 _get_dtype(1)

/Users/jreback/pandas/pandas/core/dtypes/common.py in _get_dtype(arr_or_dtype)
   1725     if hasattr(arr_or_dtype, 'dtype'):
   1726         arr_or_dtype = arr_or_dtype.dtype
-> 1727     return np.dtype(arr_or_dtype)
   1728 
   1729 

TypeError: data type not understood

In [10]: _get_dtype(np.array([1]))
Out[10]: dtype('int64')

In [11]: _get_dtype(np.array([1]).dtype)
Out[11]: dtype('int64')

In [12]: _get_dtype(Series(np.array([1])))
Out[12]: dtype('int64')
```

Commit message: Added a note for change in dtypes.common._get_dtype in whatsnew

Patch to review:
diff --git a/doc/source/whatsnew/v0.21.0.txt b/doc/source/whatsnew/v0.21.0.txt
index 015fdf1f45f47..4523569fb0f63 100644
--- a/doc/source/whatsnew/v0.21.0.txt
+++ b/doc/source/whatsnew/v0.21.0.txt
@@ -147,7 +147,8 @@ Bug Fixes
 Conversion
 ^^^^^^^^^^
 
-
+- :func:`dtypes.common._get_dtype` now accepts ``Categorical``s and
+  ``CategoricalIndex`` (:issue:`16887`).
 
 Indexing
 ^^^^^^^^
diff --git a/pandas/core/dtypes/common.py b/pandas/core/dtypes/common.py
index 2eebf3704253e..a386c04cc4fdd 100644
--- a/pandas/core/dtypes/common.py
+++ b/pandas/core/dtypes/common.py
@@ -11,7 +11,7 @@
                      ExtensionDtype)
 from .generic import (ABCCategorical, ABCPeriodIndex,
                       ABCDatetimeIndex, ABCSeries,
-                      ABCSparseArray, ABCSparseSeries)
+                      ABCSparseArray, ABCSparseSeries, ABCCategoricalIndex)
 from .inference import is_string_like
 from .inference import *  # noqa
 
@@ -1713,6 +1713,8 @@ def _get_dtype(arr_or_dtype):
             return PeriodDtype.construct_from_string(arr_or_dtype)
         elif is_interval_dtype(arr_or_dtype):
             return IntervalDtype.construct_from_string(arr_or_dtype)
+    elif isinstance(arr_or_dtype, (ABCCategorical, ABCCategoricalIndex)):
+        return arr_or_dtype.dtype
 
     if hasattr(arr_or_dtype, 'dtype'):
         arr_or_dtype = arr_or_dtype.dtype
diff --git a/pandas/tests/dtypes/test_common.py b/pandas/tests/dtypes/test_common.py
index c32e8590c5675..7188e397c0617 100644
--- a/pandas/tests/dtypes/test_common.py
+++ b/pandas/tests/dtypes/test_common.py
@@ -532,16 +532,16 @@ def test_is_complex_dtype():
     (float, np.dtype(float)),
     ('float64', np.dtype('float64')),
     (np.dtype('float64'), np.dtype('float64')),
-    pytest.mark.xfail((str, np.dtype('<U')), ),
+    (str, np.dtype(str)),
     (pd.Series([1, 2], dtype=np.dtype('int16')), np.dtype('int16')),
     (pd.Series(['a', 'b']), np.dtype(object)),
     (pd.Index([1, 2]), np.dtype('int64')),
     (pd.Index(['a', 'b']), np.dtype(object)),
     ('category', 'category'),
     (pd.Categorical(['a', 'b']).dtype, CategoricalDtype()),
-    pytest.mark.xfail((pd.Categorical(['a', 'b']), CategoricalDtype()),),
+    (pd.Categorical(['a', 'b']), CategoricalDtype()),
     (pd.CategoricalIndex(['a', 'b']).dtype, CategoricalDtype()),
-    pytest.mark.xfail((pd.CategoricalIndex(['a', 'b']), CategoricalDtype()),),
+    (pd.CategoricalIndex(['a', 'b']), CategoricalDtype()),
     (pd.DatetimeIndex([1, 2]), np.dtype('<M8[ns]')),
     (pd.DatetimeIndex([1, 2]).dtype, np.dtype('<M8[ns]')),
     ('<M8[ns]', np.dtype('<M8[ns]')),


Code review comments:
diff_hunk: @@ -147,7 +147,8 @@ Bug Fixes
 Conversion
 ^^^^^^^^^^
 
-
+- :func:`dtypes.common._get_dtype` now accepts ``Categorical``s and
path: doc/source/whatsnew/v0.21.0.txt
text: @user1:
no need for an external note here; this is an internal function

@author:
I don't quite understand. Above you say

> just needs a whatsnew note

Are you thinking of a different location for the note?

@user1:
not this doesn't need a note at all. This is not a user relevant change as ``_get_dtype`` is not user facing.