API: Add dtype parameter to Categorical.from_codes
State: closed

- [x] tests added / passed
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] whatsnew entry

Added parameter ``dtype`` to ``Categorical.from_codes``.

Labels:
name: API Design
name: Categorical
description: Categorical Data Type

Comments:
# [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/24398?src=pr&el=h1) Report
> Merging [#24398](https://codecov.io/gh/pandas-dev/pandas/pull/24398?src=pr&el=desc) into [master](https://codecov.io/gh/pandas-dev/pandas/commit/be406f316d7aa27d565894059fc609329f055972?src=pr&el=desc) will **increase** coverage by `<.01%`.
> The diff coverage is `100%`.

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

```diff
@@            Coverage Diff             @@
##           master   #24398      +/-   ##
==========================================
+ Coverage   92.37%   92.38%   +<.01%     
==========================================
  Files         166      166              
  Lines       52315    52323       +8     
==========================================
+ Hits        48327    48337      +10     
+ Misses       3988     3986       -2
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `90.8% <100%> (ø)` | :arrow_up: |
| #single | `43.06% <55.55%> (-0.01%)` | :arrow_down: |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/24398?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/core/arrays/categorical.py](https://codecov.io/gh/pandas-dev/pandas/pull/24398/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvYXJyYXlzL2NhdGVnb3JpY2FsLnB5) | `95.67% <100%> (+0.24%)` | :arrow_up: |
| [pandas/core/indexes/category.py](https://codecov.io/gh/pandas-dev/pandas/pull/24398/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvaW5kZXhlcy9jYXRlZ29yeS5weQ==) | `98.61% <100%> (ø)` | :arrow_up: |
| [pandas/core/arrays/datetimelike.py](https://codecov.io/gh/pandas-dev/pandas/pull/24398/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvYXJyYXlzL2RhdGV0aW1lbGlrZS5weQ==) | `97.67% <0%> (-0.19%)` | :arrow_down: |
| [pandas/io/formats/html.py](https://codecov.io/gh/pandas-dev/pandas/pull/24398/diff?src=pr&el=tree#diff-cGFuZGFzL2lvL2Zvcm1hdHMvaHRtbC5weQ==) | `99.34% <0%> (ø)` | :arrow_up: |
| [pandas/io/formats/format.py](https://codecov.io/gh/pandas-dev/pandas/pull/24398/diff?src=pr&el=tree#diff-cGFuZGFzL2lvL2Zvcm1hdHMvZm9ybWF0LnB5) | `97.98% <0%> (ø)` | :arrow_up: |
| [pandas/core/indexes/datetimes.py](https://codecov.io/gh/pandas-dev/pandas/pull/24398/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvaW5kZXhlcy9kYXRldGltZXMucHk=) | `96.27% <0%> (+0.01%)` | :arrow_up: |
| [pandas/core/arrays/datetimes.py](https://codecov.io/gh/pandas-dev/pandas/pull/24398/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvYXJyYXlzL2RhdGV0aW1lcy5weQ==) | `97.71% <0%> (+0.03%)` | :arrow_up: |
| [pandas/util/testing.py](https://codecov.io/gh/pandas-dev/pandas/pull/24398/diff?src=pr&el=tree#diff-cGFuZGFzL3V0aWwvdGVzdGluZy5weQ==) | `88.09% <0%> (+0.09%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/24398?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/24398?src=pr&el=footer). Last update [be406f3...0459ad0](https://codecov.io/gh/pandas-dev/pandas/pull/24398?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

# [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/24398?src=pr&el=h1) Report
> Merging [#24398](https://codecov.io/gh/pandas-dev/pandas/pull/24398?src=pr&el=desc) into [master](https://codecov.io/gh/pandas-dev/pandas/commit/3e0358d869f8a76b3223e8bb313a92edbaa369db?src=pr&el=desc) will **increase** coverage by `<.01%`.
> The diff coverage is `88.88%`.

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

```diff
@@            Coverage Diff             @@
##           master   #24398      +/-   ##
==========================================
+ Coverage    92.3%    92.3%   +<.01%     
==========================================
  Files         162      162              
  Lines       51875    51874       -1     
==========================================
+ Hits        47883    47884       +1     
+ Misses       3992     3990       -2
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `90.71% <88.88%> (ø)` | :arrow_up: |
| #single | `42.99% <77.77%> (-0.01%)` | :arrow_down: |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/24398?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/core/indexes/category.py](https://codecov.io/gh/pandas-dev/pandas/pull/24398/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvaW5kZXhlcy9jYXRlZ29yeS5weQ==) | `98.65% <100%> (ø)` | :arrow_up: |
| [pandas/core/arrays/categorical.py](https://codecov.io/gh/pandas-dev/pandas/pull/24398/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvYXJyYXlzL2NhdGVnb3JpY2FsLnB5) | `95.44% <87.5%> (+0.12%)` | :arrow_up: |
| [pandas/util/testing.py](https://codecov.io/gh/pandas-dev/pandas/pull/24398/diff?src=pr&el=tree#diff-cGFuZGFzL3V0aWwvdGVzdGluZy5weQ==) | `87.84% <0%> (+0.09%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/24398?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/24398?src=pr&el=footer). Last update [3e0358d...e2543df](https://codecov.io/gh/pandas-dev/pandas/pull/24398?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

Hello @topper-123! Thanks for updating the PR.









Cheers ! There are no PEP8 issues in this Pull Request. :beers: 

##### Comment last updated on January 08, 2019 at 14:54 Hours UTC
merge master
@topper-123 can you merge master and update
is this orthogonal to #24419 ?
Functionally yes, though I will have to solve a small merge conflict after the first of these has been merged.
ok let's merge the other first
@topper-123 can you merge master and update
I've updated, including deprecated categories & ordered parameters.

However, after #24419, it is not really a burden to support categories & ordered, and constructiong a dtype is an extra step for the user, compared to just supply categories to ``.from_codes``. Should we still deprecate?
lgtm. all of the other constructors now accept dtype so this makes consistent. @TomAugspurger @jschendel  ?
Anyone else have thoughts on deprecating the categories and ordered keywords to `Categorical.from_codes`? @jorisvandenbossche?

Reading 
https://github.com/pandas-dev/pandas/pull/24398#issuecomment-451735620, I suspect that @topper-123 may also prefer (or be OK with) not deprecating.
Yes, I’d now prefer not deprecating, from the same resoning as you (convenience).

Also, supporting categories/ordered requires very little effort now, so no real cost wrt. code complexity.
@topper-123 do you have time today to revert the deprecation?
I'll look into it.
Ok, i’ve reverted the deprecation.
Yes, +1 on not deprecating categories and ordered 
so ok with adding ``dtype`` in ``.from_codes`` as that promotes consistency, but why are folks not in favor of deprcating ``categories`` and ``ordered``? this is just moving code away from the single point of using CDT for all operations.
Mainly balancing the volume of code that is out there using categories &
orderer, against the (small) cost of supporting both.

On Tue, Jan 8, 2019 at 6:45 AM Jeff Reback <notifications@github.com> wrote:

> so ok with adding dtype in .from_codes as that promotes consistency, but
> why are folks not in favor of deprcating categories and ordered? this is
> just moving code away from the single point of using CDT for all operations.
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/pandas-dev/pandas/pull/24398#issuecomment-452285757>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/ABQHIopOGnDpPo60kgmb1BgMOvTbSueTks5vBJL8gaJpZM4ZfnHF>
> .
>

> so ok with adding dtype in .from_codes as that promotes consistency, but why are folks not in favor of deprcating categories and ordered?

We have exactly the same pattern in the main `Categorical` constructor as well: users have the option to just pass `categories` or `ordered`, or a full fledged dtype. 
Deprecating it there of course has a much larger impact than for `from_codes`, but this means we have the machinery to handle the combination, so I don't really see the need to deprecate it here (certainly given that it is much more convenient to use).
> We have exactly the same pattern in the main Categorical constructor as well: users have the option to just pass categories or ordered, or a full fledged dtype.

ok I can see the argument for this then. But this is a tag confusing, maybe let's enhance the doc-strings slightly on the constructor & from_codes to make this even more cclear that you should pass (categories, ordered) or dtype (yes it errors, but a doc-string not will help).

@topper-123 can you raise an issue / PR for this.
6008c08a331e877d554ec0400641fe9008385b4b has some changes

* added a test for raising when both categories / ordered & dtype are passed
* updated whatsnew
* updated versionchanged / added docs

Buglet when neither `categories` nor `dtype` is provided.

```python
In [1]: import pandas as pd

In [2]: pd.Categorical.from_codes([0, 1])
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-2-e8a6a967ddf5> in <module>
----> 1 pd.Categorical.from_codes([0, 1])

~/sandbox/pandas/pandas/core/arrays/categorical.py in from_codes(cls, codes, categories, ordered, dtype)
    661
    662         if len(codes) and (
--> 663                 codes.max() >= len(dtype.categories) or codes.min() < -1):
    664             raise ValueError("codes need to be between -1 and "
    665                              "len(categories)-1")

TypeError: object of type 'NoneType' has no len()

```

fixing now.
6008c08a331e877d554ec0400641fe9008385b4b also had a bug with the tests for raising when both `categories` and `dtype` were used. The test I added used `Categorical()` instead of `Categorical.from_codes`. That's fixed now.

+1 to the changes made by @TomAugspurger 
Thanks. Merging in a few hours if now objections.
yeah this is ok.
#6581 lists the categorical and ordered kwargs as deprecated, but i dont see any warnings to that effect.  Am i missing something?
That seems a mistake indeed, as those arguments are not deprecated. Looking at the last part of the comments above, there was some discussion about this but finally decided to not deprecate. 

Removed it from the list in #6581
Yeah, the original proposal was to deprecate, but we decided to keep them after all, as it we felt it was more convenient to allow a similar instantiation method as in ``__init__``.