Fixed read_csv with CategoricalDtype with boolean categories (20498) #20826

TomAugspurger · 2018-04-25T22:01:21Z

closes Force boolean column to category while reading a csv #20498
tests added / passed
passes git diff upstream/master -u -- "*.py" | flake8 --diff
whatsnew entry

pandas/core/arrays/categorical.py

codecov · 2018-04-26T11:05:13Z

Codecov Report

Merging #20826 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@            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%> (ø)`	⬆️
#single	`42.42% <0%> (-0.01%)`	⬇️

Impacted Files	Coverage Δ
pandas/io/parsers.py	`95.29% <ø> (ø)`	⬆️
pandas/core/arrays/categorical.py	`95.38% <100%> (+0.02%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e5c90e5...f96a854. Read the comment docs.

pandas/core/arrays/categorical.py

TomAugspurger · 2018-04-27T11:58:32Z

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 ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In pandas/core/arrays/categorical.py <#20826 (comment)>: > @@ -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 <#20826 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABQHIilori4wAlmJJnIwNQm7XMGDPaHpks5tswdqgaJpZM4TkOVV> .

jreback · 2018-04-27T12:00:33Z

then push to 0.23.1 (alternatively as @jorisvandenbossche mentoined, can make a 0.23rc milestone tag)

doc/source/whatsnew/v0.23.0.txt

pandas/core/arrays/categorical.py

jorisvandenbossche · 2018-06-22T07:59:59Z

@TomAugspurger do you have time to look at this one? (I don't know how far off it is)

teto · 2018-07-04T09:30:47Z

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 ?

TomAugspurger · 2018-07-04T11:09:04Z

Possibly. If you can provide a unit test I can include it or you can make a followup PR including it.

pandas/tests/categorical/test_constructors.py

doc/source/whatsnew/v0.24.0.txt

... around to_csv/read_csv and pd.api.types.CategoricalDtype for instance using pd.api.types.CategoricalDtype(categories=ConnectionRoles, ordered=True) to_csv will correctly write the column but read_csv will fail expecting a string pandas-dev/pandas#20826. I ended up writing my own converter

jreback · 2018-09-25T16:43:59Z

IIRC this was fine, can you rebase

pep8speaks · 2018-10-12T14:06:20Z

Hello @TomAugspurger! Thanks for updating the PR.

There are no PEP8 issues in the file pandas/core/arrays/categorical.py !
There are no PEP8 issues in the file pandas/io/parsers.py !
There are no PEP8 issues in the file pandas/tests/arrays/categorical/test_constructors.py !
There are no PEP8 issues in the file pandas/tests/io/parser/dtypes.py !

jreback · 2018-10-14T18:17:11Z

this looks good; haven’t looked at what is failing

jreback · 2018-11-18T22:07:51Z

@TomAugspurger can you merge master

jreback · 2018-11-23T03:20:18Z

@gfyoung if you'd like to rebase this, maybe can get this in

gfyoung · 2018-11-23T07:56:35Z

Yikes! Looks like this one got lost in the myriad of other PR's. Sure thing.

@TomAugspurger

Previously, was being parsed as object instead of boolean. Closes pandas-devgh-20498. Original Author: @TomAugspurger Rebased by @gfyoung due to merge conflicts.

gfyoung · 2018-11-23T10:44:34Z

@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).

jreback · 2018-11-23T14:48:52Z

looks ok to me. @TomAugspurger if you'd have a look.

gfyoung · 2018-11-27T16:10:03Z

@TomAugspurger : Any thoughts on this? I think we're waiting on your review before merging.

TomAugspurger · 2018-11-27T16:17:14Z

Sorry for the delay, thanks for rebasing and fixing things up.

@TomAugspurger

Previously, was being parsed as object instead of boolean. Closes pandas-devgh-20498. Original Author: @TomAugspurger Rebased by @gfyoung due to merge conflicts.

@TomAugspurger

Previously, was being parsed as object instead of boolean. Closes pandas-devgh-20498. Original Author: @TomAugspurger Rebased by @gfyoung due to merge conflicts.

TomAugspurger added IO CSV read_csv, to_csv Categorical Categorical Data Type labels Apr 25, 2018

TomAugspurger added this to the 0.23.0 milestone Apr 25, 2018

TomAugspurger force-pushed the categorical-bool-fixed branch from 6b5cc4f to 5495551 Compare April 25, 2018 22:01

TomAugspurger commented Apr 25, 2018

View reviewed changes

pandas/core/arrays/categorical.py Outdated Show resolved Hide resolved

TomAugspurger changed the title ~~Fixed read_csv with CategoricalDtype with boolena categories (20498)~~ Fixed read_csv with CategoricalDtype with boolean categories (20498) Apr 26, 2018

jreback requested changes Apr 27, 2018

View reviewed changes

pandas/core/arrays/categorical.py Show resolved Hide resolved

jreback modified the milestones: 0.23.0, 0.23.1 Apr 27, 2018

jreback requested changes Jun 4, 2018

View reviewed changes

doc/source/whatsnew/v0.23.0.txt Outdated Show resolved Hide resolved

pandas/core/arrays/categorical.py Outdated Show resolved Hide resolved

pandas/core/arrays/categorical.py Outdated Show resolved Hide resolved

jreback removed this from the 0.23.1 milestone Jun 4, 2018

jreback requested changes Jul 5, 2018

View reviewed changes

pandas/tests/categorical/test_constructors.py Outdated Show resolved Hide resolved

doc/source/whatsnew/v0.24.0.txt Outdated Show resolved Hide resolved

jreback added this to the 0.24.0 milestone Jul 5, 2018

BUG: Properly handle CSV boolean CategoricalDtype

f96a854

Previously, was being parsed as object instead of boolean. Closes pandas-devgh-20498. Original Author: @TomAugspurger Rebased by @gfyoung due to merge conflicts.

gfyoung force-pushed the categorical-bool-fixed branch from a1f64ff to f96a854 Compare November 23, 2018 10:39

jreback approved these changes Nov 23, 2018

View reviewed changes

TomAugspurger merged commit d53a4cc into pandas-dev:master Nov 27, 2018

TomAugspurger deleted the categorical-bool-fixed branch November 27, 2018 16:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixed read_csv with CategoricalDtype with boolean categories (20498) #20826

Fixed read_csv with CategoricalDtype with boolean categories (20498) #20826

TomAugspurger commented Apr 25, 2018

codecov bot commented Apr 26, 2018 •

edited

Loading

TomAugspurger commented Apr 27, 2018 via email

jreback commented Apr 27, 2018

jorisvandenbossche commented Jun 22, 2018

teto commented Jul 4, 2018

TomAugspurger commented Jul 4, 2018

jreback commented Sep 25, 2018

pep8speaks commented Oct 12, 2018

jreback commented Oct 14, 2018

jreback commented Nov 18, 2018

jreback commented Nov 23, 2018

gfyoung commented Nov 23, 2018

gfyoung commented Nov 23, 2018 •

edited

Loading

jreback commented Nov 23, 2018

gfyoung commented Nov 27, 2018

TomAugspurger commented Nov 27, 2018

Fixed read_csv with CategoricalDtype with boolean categories (20498) #20826

Fixed read_csv with CategoricalDtype with boolean categories (20498) #20826

Conversation

TomAugspurger commented Apr 25, 2018

codecov bot commented Apr 26, 2018 • edited Loading

Codecov Report

TomAugspurger commented Apr 27, 2018 via email

jreback commented Apr 27, 2018

jorisvandenbossche commented Jun 22, 2018

teto commented Jul 4, 2018

TomAugspurger commented Jul 4, 2018

jreback commented Sep 25, 2018

pep8speaks commented Oct 12, 2018

jreback commented Oct 14, 2018

jreback commented Nov 18, 2018

jreback commented Nov 23, 2018

gfyoung commented Nov 23, 2018

gfyoung commented Nov 23, 2018 • edited Loading

jreback commented Nov 23, 2018

gfyoung commented Nov 27, 2018

TomAugspurger commented Nov 27, 2018

codecov bot commented Apr 26, 2018 •

edited

Loading

gfyoung commented Nov 23, 2018 •

edited

Loading