Skip to content

Allow dtype='category' or dtype=None for Series with Categorical #12636

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
wants to merge 5 commits into from
Closed

Allow dtype='category' or dtype=None for Series with Categorical #12636

wants to merge 5 commits into from

Conversation

drewfustin
Copy link
Contributor

Allows user to pass dtype='category' when creating a Series with Categorical, but produces a ValueError is any non-categorical dtype is passed.

TomAugspurger
TomAugspurger reviewed Mar 16, 2016
View reviewed changes
pandas/tests/test_dtypes.py Outdated
self.assertFalse(is_categorical(1.0))

def test_series_with_dtype(self):
self.assertRaises(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a comment like #GH 12574, pointing back to the original issue?

@TomAugspurger
Copy link
Contributor

Just the one comment, +1 otherwise (assuming Travis passes). Thanks!

@TomAugspurger TomAugspurger added the Categorical Categorical Data Type label Mar 16, 2016
@TomAugspurger TomAugspurger added this to the 0.18.1 milestone Mar 16, 2016
@TomAugspurger TomAugspurger added the Dtype Conversions Unexpected or buggy dtype conversions label Mar 16, 2016
jreback
jreback reviewed Mar 16, 2016
View reviewed changes
pandas/core/series.py
elif isinstance(data, Categorical):
if dtype is not None:
# Allow dtype=category only, otherwise error
if ((dtype is not None) and
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is not right here; use is_categorical_dtype(dtype)

@codecov-io
Copy link

Current coverage is 88.64%

Merging #12636 into master will not affect coverage as of 724c309

@@            master   #12636   diff @@
=======================================
  Files          275      275       
  Stmts       133688   133695     +7
  Branches         0        0       
  Methods          0        0       
=======================================
+ Hit         118511   118520     +9
  Partial          0        0       
+ Missed       15177    15175     -2

Review entire Coverage Diff as of 724c309

Powered by Codecov. Updated on successful CI builds.

@jreback jreback closed this in a5670f2 Mar 16, 2016
@jreback
Copy link
Contributor

jreback commented Mar 16, 2016

thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

Categorical Categorical Data Type Dtype Conversions Unexpected or buggy dtype conversions

Projects

None yet

Milestone

0.18.1

Development

Successfully merging this pull request may close these issues.

BUG: Series with Categorical and dtype

4 participants

@drewfustin @TomAugspurger @codecov-io @jreback