BUG: Fix is_categorical_dtype for Sparse[category]
State: closed

- [x] closes #35793
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] whatsnew entry


Labels:
name: Dtype Conversions
description: Unexpected or buggy dtype conversions
name: Sparse
description: Sparse Data Type
name: Categorical
description: Categorical Data Type

Comments:
Ran the categoricals.py asvs and somehow performance improved (?):
```
       before           after         ratio
     [97ec0627]       [b52a7eab]
     <master>         <sparse-categorical-dtype>
-      10.0±0.3ms       9.05±0.2ms     0.90  categoricals.Isin.time_isin_categorical('int64')
-      13.5±0.2ms         11.9±1ms     0.88  categoricals.Isin.time_isin_categorical('object')
-        5.75±2μs       4.00±0.1μs     0.69  categoricals.Indexing.time_get_loc

SOME BENCHMARKS HAVE CHANGED SIGNIFICANTLY.
PERFORMANCE INCREASED.
```
mypy error seems wrong, dtype.subtype comes after a hasattr(dtype, "subtype") check:
```
pandas/core/dtypes/base.py:292: error: "object" has no attribute "subtype"  [attr-defined]
```
Any ideas on how best to fix? Could explicitly ignore or maybe use something like isinstance(getattr(dtype, "subtype", None), cls)
> Is this change actually something we necessarily want?
> 
> For example one use case where you might check that something is a categorical, is to know that you can then use certain APIs specific to categoricals. But if a Sparse type also gives true, that won't longer be true.

Hmm, interesting point. I think the argument from the original issue seems valid though, that if Sparse[int] is an int, then Sparse[categorical] should be a categorical (making something sparse shouldn't change that). I suppose adopting that view would mean that there's really less of an API specific to categoricals, so some special casing might have to happen as a result.

I can see the point though, that this could just make things more confusing in other ways.
Yes, but as I noted on the issue: if there is an inconsistency, there are always two ways this can be solved. Meaning: we can also change Sparse[int] to no longer be int, or at least check what the use cases are for it being considered int (there quite possibly are, but I think we should try to be concrete first before deciding how to solve the inconsistency)
> Meaning: we can also change Sparse[int] to no longer be int, or at least check what the use cases are for it being considered int (there quite possibly are, but I think we should try to be concrete first before deciding how to solve the inconsistency)

The more I think about it the more this seems like the better option over changing this for categorical; is_categorical_dtype loses most of it's usefulness if it doesn't guarantee access to the API specific to categoricals.

So then to be consistent sparse dtypes should really only register as sparse (not int, float, whatever)? I would be curious if that's actually easier internally, or what that might break if changed (or if you even bother changing it at all).
> mypy error seems wrong, dtype.subtype comes after a hasattr(dtype, "subtype") check:
> 
> ```
> pandas/core/dtypes/base.py:292: error: "object" has no attribute "subtype"  [attr-defined]
> ```
> 
> Any ideas on how best to fix? Could explicitly ignore or maybe use something like isinstance(getattr(dtype, "subtype", None), cls)

see https://github.com/python/mypy/issues/1424

use `#type: ignore` and add a comment referencing the mypy issue
@jorisvandenbossche One possible counterpoint is that is_categorical_dtype already doesn't guarantee access to the categorical API:

```python
[ins] In [1]: import pandas as pd

[ins] In [2]: from pandas.core.dtypes.common import is_categorical_dtype

[ins] In [3]: s = pd.Series(pd.Categorical(["a", "b", "c"]))

[ins] In [4]: is_categorical_dtype(s)
Out[4]: True

[ins] In [5]: s.codes
---------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)
<ipython-input-5-c9d8be9c51ce> in <module>
----> 1 s.codes

~/opt/miniconda3/envs/pandas-1.1.2/lib/python3.8/site-packages/pandas/core/generic.py in __getattr__(self, name)
   5134             if self._info_axis._can_hold_identifiers_and_holds_name(name):
   5135                 return self[name]
-> 5136             return object.__getattribute__(self, name)
   5137
   5138     def __setattr__(self, name: str, value) -> None:

AttributeError: 'Series' object has no attribute 'codes'
```

That assumption caused a bug here: https://github.com/pandas-dev/pandas/issues/36383
Hello @dsaxton! 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-09-19 13:01:13 UTC
code change looks ok. but I think @jorisvandenbossche is right here; if `is_categorical_dtype` is true then this should *act* like a categorical, which we currently don't do.

> One possible counterpoint is that is_categorical_dtype already doesn't guarantee access to the categorical API:

But it does guarantee that on a Series `.cat.codes` is available (it's true you still need to know if it's a Series[Categorical] or Categorical).

Also, do we actually support Sparse[category] properly? 
Eg I just took the example from the test you added, and the repr fails:

```
In [1]: s = pd.Series(["a", "b", "c"], dtype=pd.SparseDtype(pd.CategoricalDtype(["a", "b", "c"])))                                                                                                                 

In [2]: s                                                                                                                                                                                                          
...
ValueError: object __array__ method not producing an array
```

Some time ago I opened a general issue about the question to support Sparse backed by other ExtensionArrays or not: https://github.com/pandas-dev/pandas/issues/26407
Closing since it seems there isn't a desire to change this