BUG: Ensure Index.astype('category') returns a CategoricalIndex
State: closed

- [X] closes #18630
- [X] tests added / passed
- [X] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [X] whatsnew entry

Notes:
  - `MultiIndex.astype('category')` raises per @TomAugspurger's comment in the issue.
  - `IntervalIndex.astype('category')` return a `Categorical` with `ordered=True` instead of `CategoricalIndex`, since it looks like someone previously intentionally implemented it this way.  I don't immediately see a reason why, but left it as is.  Would be straightforward to make this consistent and return a `CategoricalIndex`.
  -  All other types of index should return a `CategoricalIndex`.

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

Comments:
> IntervalIndex.astype('category') return a Categorical with ordered=True instead of CategoricalIndex, since it looks like someone previously intentionally implemented it this way.

I think it should be CategoricalIndex
# [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/18677?src=pr&el=h1) Report
> Merging [#18677](https://codecov.io/gh/pandas-dev/pandas/pull/18677?src=pr&el=desc) into [master](https://codecov.io/gh/pandas-dev/pandas/commit/ae74c2b11580af2e6636f24871e0bc331fd4fad5?src=pr&el=desc) will **decrease** coverage by `0.01%`.
> The diff coverage is `100%`.

[![Impacted file tree graph](https://codecov.io/gh/pandas-dev/pandas/pull/18677/graphs/tree.svg?width=650&src=pr&token=eZ4WkYLtcO&height=150)](https://codecov.io/gh/pandas-dev/pandas/pull/18677?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master   #18677      +/-   ##
==========================================
- Coverage    91.6%   91.59%   -0.02%     
==========================================
  Files         153      153              
  Lines       51306    51339      +33     
==========================================
+ Hits        46998    47022      +24     
- Misses       4308     4317       +9
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `89.45% <100%> (ø)` | :arrow_up: |
| #single | `40.74% <31.7%> (-0.11%)` | :arrow_down: |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/18677?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/core/indexes/multi.py](https://codecov.io/gh/pandas-dev/pandas/pull/18677/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvaW5kZXhlcy9tdWx0aS5weQ==) | `96.29% <100%> (+0.01%)` | :arrow_up: |
| [pandas/core/dtypes/dtypes.py](https://codecov.io/gh/pandas-dev/pandas/pull/18677/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZHR5cGVzL2R0eXBlcy5weQ==) | `95.27% <100%> (+0.13%)` | :arrow_up: |
| [pandas/core/indexes/numeric.py](https://codecov.io/gh/pandas-dev/pandas/pull/18677/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvaW5kZXhlcy9udW1lcmljLnB5) | `97.36% <100%> (+0.03%)` | :arrow_up: |
| [pandas/core/indexes/interval.py](https://codecov.io/gh/pandas-dev/pandas/pull/18677/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvaW5kZXhlcy9pbnRlcnZhbC5weQ==) | `93.8% <100%> (ø)` | :arrow_up: |
| [pandas/core/indexes/datetimes.py](https://codecov.io/gh/pandas-dev/pandas/pull/18677/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvaW5kZXhlcy9kYXRldGltZXMucHk=) | `95.7% <100%> (+0.01%)` | :arrow_up: |
| [pandas/core/indexes/base.py](https://codecov.io/gh/pandas-dev/pandas/pull/18677/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvaW5kZXhlcy9iYXNlLnB5) | `96.43% <100%> (ø)` | :arrow_up: |
| [pandas/core/indexes/category.py](https://codecov.io/gh/pandas-dev/pandas/pull/18677/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvaW5kZXhlcy9jYXRlZ29yeS5weQ==) | `97.23% <100%> (+0.03%)` | :arrow_up: |
| [pandas/core/indexes/period.py](https://codecov.io/gh/pandas-dev/pandas/pull/18677/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvaW5kZXhlcy9wZXJpb2QucHk=) | `92.93% <100%> (+0.03%)` | :arrow_up: |
| [pandas/core/dtypes/common.py](https://codecov.io/gh/pandas-dev/pandas/pull/18677/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZHR5cGVzL2NvbW1vbi5weQ==) | `94.45% <100%> (ø)` | :arrow_up: |
| [pandas/core/indexes/timedeltas.py](https://codecov.io/gh/pandas-dev/pandas/pull/18677/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvaW5kZXhlcy90aW1lZGVsdGFzLnB5) | `91.26% <100%> (+0.05%)` | :arrow_up: |
| ... and [2 more](https://codecov.io/gh/pandas-dev/pandas/pull/18677/diff?src=pr&el=tree-more) | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/18677?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/18677?src=pr&el=footer). Last update [ae74c2b...6042131](https://codecov.io/gh/pandas-dev/pandas/pull/18677?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

Question regarding how this should behave with a `CategoricalIndex`.

Setup:
```python
In [3]: ci = pd.CategoricalIndex(list('abca'))

In [4]: new_dtype = CategoricalDtype(ordered=True)

In [5]: new_dtype
Out[5]: CategoricalDtype(categories=None, ordered=True)

In [6]: ci.dtype
Out[6]: CategoricalDtype(categories=['a', 'b', 'c'], ordered=False)
```

Then `ci.dtype` and `new_dtype` are considered equal despite `ordered` being different:
```python
In [7]: ci.dtype == new_dtype
Out[7]: True
```
Which appears to be intentional, per the comment prior to the implementation:

https://github.com/pandas-dev/pandas/blob/9629fef912dbeca5154e9cf58970bad3e02657f4/pandas/core/dtypes/dtypes.py#L216-L222

In this case, should doing `ci.astype(new_dtype)` change `ordered` to `True`?
  - From a user standpoint I'd expect this to change `ordered` to `True`, as this would logically be the user's intention.
  - From a codebase consistency standpoint, it seems like since the two dtypes are equal, doing an `astype` shouldn't change anything
Made review related updates and a few additional fixes/changes:
  - `IntervalIndex.astype('category')` has been changed to return a `CategoricalIndex`
  - Modified `MultiIndex.astype('category')` to raise a `NotImplementedError` stating that >1 ndim categoricals aren't currently supported
      - Mirrors a similar error message raised by `DataFrame.astype('category')`.

Also modified the `CategoricalIndex.astype` behavior to address my question above.  I chose the first bulleted option.  The logic I implemented is that if a `CategoricalDtype` with `categories` or `ordered` being `None` is passed, `None` is replaced by the corresponding value from the `CategoricalIndex`'s dtype.  Basically `None` corresponds to "don't change this attribute".

The only slightly counter-intuitive thing is that `pandas_dtype('category')` returns a `CategoricalDtype` with `ordered=False`, meaning that if you have an ordered `CategoricalIndex` and do `.astype('category')`  it would keep the same categories but switch ordered to `False`.  Should be straightforward to make this not change anything if that'd be preferred.
> should doing ci.astype(new_dtype) change ordered to True?

I agree that from a user point of view this should change the ordered attribute.

> From a codebase consistency standpoint, it seems like since the two dtypes are equal, doing an astype shouldn't change anything

I am wondering if it would not be possible to change this. It is clear that we need to keep `cat_dtype == 'category'` to be True. But that is already handled a few lines above the snippet you showed. 
So I don't fully understand the comment why it is needed that all CDT(None, ordered=True/False) needs to be equal regardless of the ordered attribute.

cc @TomAugspurger 

>  if you have an ordered CategoricalIndex and do .astype('category') it would keep the same categories but switch ordered to False. Should be straightforward to make this not change anything if that'd be preferred.

That doesn't sound fully as the desired behaviour .. although I am not fully sure :-)
> >  if you have an ordered CategoricalIndex and do .astype('category') it would keep the same categories but switch ordered to False. Should be straightforward to make this not change anything if that'd be preferred.

> That doesn't sound fully as the desired behaviour .. although I am not fully sure :-)

Yes, agreed that it's unclear. In that case, I think it's less surprising for `'category'` to behave as `CDT(None, None)`, i.e. don't change anything.

>  So I don't fully understand the comment why it is needed that all CDT(None, ordered=True/False) needs to be equal regardless of the ordered attribute.

That was for backwards comparability. I wouldn't use it as an argument of consistency, since it's a bad thing to be consistent about :)
Updates:
  - Modified `CategoricalIndex.astype('category')` to not change anything
  -  Fixed some tests that previously relied on `IntervalIndex.astype('category')` returning a `Categorical` with `ordered=True` instead of a `CategoricalIndex`
      - This only impacted the creation of `expected` in the tests; didn't change `result`.
Updated to move the `CI` imports, and replied to the other comments in the latest review.
Looks good to me
Moved the categorical dtype update code to a new `_update_dtype` method of `CategoricalDtype`, per  https://github.com/pandas-dev/pandas/pull/18710#discussion_r155957618

Not sure if the name/location is appropriate, but can rename/move if need be.  For the time being, I've maintained the existing logic of `.astype('category')` not updating anything. 
thanks @jschendel nice patch! keep em coming!