Skip to content

Let _get_dtype accept Categoricals and CategoricalIndex #16887

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

Merged
merged 4 commits into from
Jul 13, 2017
Merged

Let _get_dtype accept Categoricals and CategoricalIndex #16887

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7cf00a2
Make _get_dtype accept pd.Categorical and pd.CategoricalIndex instances
Jul 11, 2017
fb0fe34
small flake8 cleanup
Jul 11, 2017
3b4ef99
Added a note for change in dtypes.common._get_dtype in whatsnew
Jul 12, 2017
54da145
Removed entry in whatsnew
topper-123 Jul 13, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion doc/source/whatsnew/v0.21.0.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,8 @@ Bug Fixes
Conversion
^^^^^^^^^^


- :func:`dtypes.common._get_dtype` now accepts ``Categorical``s and
Copy link
Contributor

Choose a reason for hiding this comment

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

no need for an external note here; this is an internal function

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't quite understand. Above you say

just needs a whatsnew note

Are you thinking of a different location for the note?

Copy link
Contributor

Choose a reason for hiding this comment

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

not this doesn't need a note at all. This is not a user relevant change as _get_dtype is not user facing.

``CategoricalIndex`` (:issue:`16887`).

Indexing
^^^^^^^^
Expand Down
4 changes: 3 additions & 1 deletion pandas/core/dtypes/common.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
ExtensionDtype)
from .generic import (ABCCategorical, ABCPeriodIndex,
ABCDatetimeIndex, ABCSeries,
ABCSparseArray, ABCSparseSeries)
ABCSparseArray, ABCSparseSeries, ABCCategoricalIndex)
from .inference import is_string_like
from .inference import * # noqa

Expand Down Expand Up @@ -1713,6 +1713,8 @@ def _get_dtype(arr_or_dtype):
return PeriodDtype.construct_from_string(arr_or_dtype)
elif is_interval_dtype(arr_or_dtype):
return IntervalDtype.construct_from_string(arr_or_dtype)
elif isinstance(arr_or_dtype, (ABCCategorical, ABCCategoricalIndex)):
return arr_or_dtype.dtype

if hasattr(arr_or_dtype, 'dtype'):
arr_or_dtype = arr_or_dtype.dtype
Expand Down
6 changes: 3 additions & 3 deletions pandas/tests/dtypes/test_common.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -532,16 +532,16 @@ def test_is_complex_dtype():
(float, np.dtype(float)),
('float64', np.dtype('float64')),
(np.dtype('float64'), np.dtype('float64')),
pytest.mark.xfail((str, np.dtype('<U')), ),
(str, np.dtype(str)),
Copy link
Contributor

Choose a reason for hiding this comment

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

what fixed this one?

Copy link
Contributor Author

@topper-123 topper-123 Jul 12, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

On both my system (Windows) and at try.jupyter.org (sys.platform is 'linux'), _get_dtype(str) returns np.dtype('<U'). In the jenkins/appvoyeur, it returns np.dtype('S').

To me it looks like a difference of how numpy is handled on different system and not a pandas issue. I therefore relax that test, to make it only check that _get_dtype(str) == np.dtype(str), which passes both locally and in the CI system.

Copy link
Contributor

Choose a reason for hiding this comment

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

hmm, that looks like a py2/py3 thing. ok then. (you can maybe add a comment for that particular case)

(pd.Series([1, 2], dtype=np.dtype('int16')), np.dtype('int16')),
(pd.Series(['a', 'b']), np.dtype(object)),
(pd.Index([1, 2]), np.dtype('int64')),
(pd.Index(['a', 'b']), np.dtype(object)),
('category', 'category'),
(pd.Categorical(['a', 'b']).dtype, CategoricalDtype()),
pytest.mark.xfail((pd.Categorical(['a', 'b']), CategoricalDtype()),),
(pd.Categorical(['a', 'b']), CategoricalDtype()),
(pd.CategoricalIndex(['a', 'b']).dtype, CategoricalDtype()),
pytest.mark.xfail((pd.CategoricalIndex(['a', 'b']), CategoricalDtype()),),
(pd.CategoricalIndex(['a', 'b']), CategoricalDtype()),
(pd.DatetimeIndex([1, 2]), np.dtype('<M8[ns]')),
(pd.DatetimeIndex([1, 2]).dtype, np.dtype('<M8[ns]')),
('<M8[ns]', np.dtype('<M8[ns]')),
Expand Down