Fixed read_csv with CategoricalDtype with boolean categories (20498)
State: closed

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

Labels:
name: IO CSV
description: read_csv, to_csv
name: Categorical
description: Categorical Data Type

Comments:
# [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/20826?src=pr&el=h1) Report
> Merging [#20826](https://codecov.io/gh/pandas-dev/pandas/pull/20826?src=pr&el=desc) into [master](https://codecov.io/gh/pandas-dev/pandas/commit/e5c90e5e355077c4ee5245a4a72833f4e8af72b9?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/20826/graphs/tree.svg?width=650&token=eZ4WkYLtcO&height=150&src=pr)](https://codecov.io/gh/pandas-dev/pandas/pull/20826?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master   #20826      +/-   ##
==========================================
+ Coverage   92.29%   92.29%   +<.01%     
==========================================
  Files         161      161              
  Lines       51497    51502       +5     
==========================================
+ Hits        47530    47535       +5     
  Misses       3967     3967
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `90.69% <100%> (ø)` | :arrow_up: |
| #single | `42.42% <0%> (-0.01%)` | :arrow_down: |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/20826?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/io/parsers.py](https://codecov.io/gh/pandas-dev/pandas/pull/20826/diff?src=pr&el=tree#diff-cGFuZGFzL2lvL3BhcnNlcnMucHk=) | `95.29% <ø> (ø)` | :arrow_up: |
| [pandas/core/arrays/categorical.py](https://codecov.io/gh/pandas-dev/pandas/pull/20826/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvYXJyYXlzL2NhdGVnb3JpY2FsLnB5) | `95.38% <100%> (+0.02%)` | :arrow_up: |

------

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

I don't think I'll have time before the RC, and we're simply fixing a bug
in the existing code.

I'd like to keep the just merge bug fixes with the RC before the final, so
your call if you want to push to 0.23.1.

On Fri, Apr 27, 2018 at 6:56 AM, Jeff Reback <notifications@github.com>
wrote:

> *@jreback* commented on this pull request.
> ------------------------------
>
> In pandas/core/arrays/categorical.py
> <https://github.com/pandas-dev/pandas/pull/20826#discussion_r184666226>:
>
> > @@ -523,6 +527,11 @@ def _from_inferred_categories(cls, inferred_categories, inferred_codes,
>                  cats = to_datetime(inferred_categories, errors='coerce')
>              elif is_timedelta64_dtype(dtype.categories):
>                  cats = to_timedelta(inferred_categories, errors='coerce')
> +            elif dtype.categories.is_boolean():
>
> this isn't necessary for the RC. this needs to be fixed in this PR.
>
> —
> You are receiving this because you authored the thread.
> Reply to this email directly, view it on GitHub
> <https://github.com/pandas-dev/pandas/pull/20826#discussion_r184666226>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/ABQHIilori4wAlmJJnIwNQm7XMGDPaHpks5tswdqgaJpZM4TkOVV>
> .
>

then push to 0.23.1 (alternatively as @jorisvandenbossche mentoined, can make a 0.23rc milestone tag)
@TomAugspurger do you have time to look at this one? (I don't know how far off it is)
With pandas 0.23, I have the following  code where the csv is correctly generated (to_csv) but the read_csv (with appropriate dtypes) will fill the column with NaN
```
class ConnectionRoles(Enum):
    Client = 0
    Server = 1

dtype_role = pd.api.types.CategoricalDtype(categories=ConnectionRoles, ordered=True)
```
does this PR address that ?
Possibly. If you can provide a unit test I can include it or you can make a followup PR including it.
IIRC this was fine, can you rebase
Hello @TomAugspurger! Thanks for updating the PR.

 - There are no PEP8 issues in the file [`pandas/core/arrays/categorical.py`](https://github.com/TomAugspurger/pandas/blob/a1f64ff96ef2a4e9f84cad10733e073a5ad99e1c/pandas/core/arrays/categorical.py) !

 - There are no PEP8 issues in the file [`pandas/io/parsers.py`](https://github.com/TomAugspurger/pandas/blob/a1f64ff96ef2a4e9f84cad10733e073a5ad99e1c/pandas/io/parsers.py) !

 - There are no PEP8 issues in the file [`pandas/tests/arrays/categorical/test_constructors.py`](https://github.com/TomAugspurger/pandas/blob/a1f64ff96ef2a4e9f84cad10733e073a5ad99e1c/pandas/tests/arrays/categorical/test_constructors.py) !

 - There are no PEP8 issues in the file [`pandas/tests/io/parser/dtypes.py`](https://github.com/TomAugspurger/pandas/blob/a1f64ff96ef2a4e9f84cad10733e073a5ad99e1c/pandas/tests/io/parser/dtypes.py) !


this looks good; haven’t looked at what is failing
@TomAugspurger can you merge master
@gfyoung if you'd like to rebase this, maybe can get this in
Yikes!  Looks like this one got lost in the myriad of other PR's.  Sure thing.
@jreback  @TomAugspurger : Rebased this PR.  Wanted to note a couple of things (also helps to address many of the outstanding comments that were still current):

* I personally was not a fan of moving an entire `Categorical` method into `parsers.pyx`.  That IMO seems to do the kind of conflating that was pushed back against earlier.  I was able to get things to work without making this migration.  At the same time, the "parsing" of `true_values` is relegated to `parsers.pyx` before being passed to `Categorical._from_inferred_categories`.

One extra thing I had to do was check for `CategoricalDtype` before attempting to do `dtype` inference with other types (e.g. `bool`, `int`).  This was needed to get all engines to pass.

* I'm okay with having a single `whatsnew` entry line.  I reworded it a little to make things clearer.

* Not sure we need a version added on the docstring, given that `Categorical._from_inferred_categories` is a private method by naming (starts with an underscore).
looks ok to me. @TomAugspurger if you'd have a look.
@TomAugspurger : Any thoughts on this?  I think we're waiting on your review before merging.
Sorry for the delay, thanks for rebasing and fixing things up.