Skip to content

Support for use of Enums in MultiIndex #21348

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

Closed
wants to merge 25 commits into from
Closed

Support for use of Enums in MultiIndex #21348

Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
0ec3531
Use of Enums in MultiIndex now supported
benediamond Jun 6, 2018
ca40575
added a test for `MultiIndex` with `Enum`
benediamond Jun 7, 2018
3857229
Update test_multi.py
benediamond Jun 7, 2018
9aee04a
PEP8
benediamond Jun 7, 2018
60c8f2a
Put imports on top
benediamond Jun 7, 2018
13d67ca
Update appveyor-27.yaml
benediamond Jun 7, 2018
e6e6627
Update circle-27-compat.yaml
benediamond Jun 7, 2018
d0607f4
Update travis-27-locale.yaml
benediamond Jun 7, 2018
ec5900a
Update travis-27.yaml
benediamond Jun 7, 2018
979b61e
enum34 dependency
benediamond Jun 7, 2018
1c84176
add whatsnew
benediamond Jun 7, 2018
e914b20
sync whatsnew with master
benediamond Jun 7, 2018
5a55fee
sync whatsnew with master
benediamond Jun 7, 2018
191fe58
attempting to resolve conflict
benediamond Jun 7, 2018
bac47e2
attempting to resolve conflicts in `whatsnew`
benediamond Jun 7, 2018
f76d522
remove whatsnew
benediamond Jun 8, 2018
f125bf7
remove whatsnew
benediamond Jun 8, 2018
761a9d1
add to whatsnew 0.24
benediamond Jun 8, 2018
2e7921b
remove nested function from test
benediamond Jun 8, 2018
21014c6
move imports to top
benediamond Jun 8, 2018
4cd2dff
jreback fixes
benediamond Jun 19, 2018
702594b
import pandas.util._test_decorators as td
benediamond Aug 12, 2018
1950562
rebase
benediamond Nov 22, 2018
6e4485f
added test
benediamond Nov 22, 2018
06437cd
typo: remove 'self'
benediamond Nov 22, 2018
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
1 change: 1 addition & 0 deletions ci/appveyor-27.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,3 +27,4 @@ dependencies:
- pytest
- pytest-xdist
- moto
- enum34
1 change: 1 addition & 0 deletions ci/circle-27-compat.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ dependencies:
# universal
- pytest
- pytest-xdist
- enum34
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 remove from all but 1 of the 27 compat tests (the travis-27) one

Copy link
Author

Choose a reason for hiding this comment

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

done

- pip:
- html5lib==1.0b2
- beautifulsoup4==4.2.1
Expand Down
1 change: 1 addition & 0 deletions ci/travis-27-locale.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ dependencies:
# universal
- pytest
- pytest-xdist
- enum34
- pip:
- html5lib==1.0b2
- beautifulsoup4==4.2.1
1 change: 1 addition & 0 deletions ci/travis-27.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ dependencies:
- pytest
- pytest-xdist
- moto
- enum34
- pip:
- backports.lzma
- cpplint
Expand Down
2 changes: 1 addition & 1 deletion doc/source/whatsnew/v0.24.0.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ Strings
Indexing
^^^^^^^^

-
- Bug in :func:`pandas.core.arrays.categorical._factorize_from_iterable` inappropriately caused ``TypeError`` to be raised when an ``Enum`` was used as a factor in a ``MultiIndex`` (:issue:`21298`)
Copy link
Contributor

Choose a reason for hiding this comment

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

Release notes shouldn't reference private methods.

You should be able to reference Enum with :class:enum.Enum, and :class:MultiIndex.

-
-

Expand Down
2 changes: 1 addition & 1 deletion pandas/core/arrays/categorical.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2523,7 +2523,7 @@ def _factorize_from_iterable(values):
ordered=values.ordered)
codes = values.codes
else:
cat = Categorical(values, ordered=True)
cat = Categorical(values, ordered=False)
Copy link
Member

Choose a reason for hiding this comment

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

Interesting that this didn't cause any testing issues...

Copy link
Author

@benediamond benediamond Jun 9, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Yes, it's striking. It's possible that thatindexes.multi is essentially the only caller of categorical._factorize_from_iterable, and that the calls that multi makes happen to be such that within Categorical's constructor, control is always routed so that the only time that ordered is used is to incorrectly raise the TypeError on lines 352-354. That's my best-guess explanation, at least.

categories = cat.categories
codes = cat.codes
return codes, categories
Expand Down
28 changes: 27 additions & 1 deletion pandas/tests/indexes/test_multi.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

from datetime import timedelta
from itertools import product

from enum import Enum
import pytest

import numpy as np
Expand All @@ -20,6 +20,7 @@
from pandas.core.indexes.base import InvalidIndexError
from pandas.core.dtypes.cast import construct_1d_object_array_from_listlike
from pandas._libs.tslib import Timestamp
from pandas.core.algorithms import take_1d

import pandas.util.testing as tm

Expand Down Expand Up @@ -3307,3 +3308,28 @@ def test_duplicate_multiindex_labels(self):
with pytest.raises(ValueError):
ind.set_levels([['A', 'B', 'A', 'A', 'B'], [2, 1, 3, -2, 5]],
inplace=True)

def test_use_enum_in_multiindex(self):
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 use @td.skip_if_no('enum') here (then do the actual import inside), we have to guard against the tests being run on 2.7 w/o the compat library (by a user).

Copy link
Author

Choose a reason for hiding this comment

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

https://github.com/benediamond/pandas/blob/4cd2dff96775a828687acd7c65919ec135824cf4/pandas/tests/indexes/test_multi.py#L3310-L3314

# GH 21298
# Allow use of Enums as one of the factors in a MultiIndex.
MyEnum = Enum("MyEnum", "A B")
Copy link
Contributor

Choose a reason for hiding this comment

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

you would have to add enum34 (the 2.7 backport) in our 2.7 builds

(pandas) bash-3.2$ grep python=2.7 ci/*.yaml
appveyor-27.yaml:  - python=2.7.*
circle-27-compat.yaml:  - python=2.7*
travis-27-locale.yaml:  - python=2.7
travis-27.yaml:  - python=2.7*

Copy link
Author

Choose a reason for hiding this comment

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

ok, I've hopefully done this correctly.

df = pd.DataFrame(columns=pd.MultiIndex.from_product(iterables=[
MyEnum,
[1, 2]
]))

# cf. https://github.com/pandas-dev/pandas/blob/
# 0c65c57a279e755ab7093db925d1e580f9878dae/pandas/util/testing.py#L793-L799
unique = df.columns.levels[0]
labels = df.columns.labels[0]
filled = take_1d(unique.values, labels, fill_value=unique._na_value)
df_index_0 = unique._shallow_copy(filled, name=df.columns.names[0])
exp_index_0 = pd.Index([MyEnum.A, MyEnum.A, MyEnum.B, MyEnum.B],
dtype='object')
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 pretty complicated, why cannot you just directly construct the Index here?

Copy link
Author

Choose a reason for hiding this comment

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

https://github.com/benediamond/pandas/blob/4cd2dff96775a828687acd7c65919ec135824cf4/pandas/tests/indexes/test_multi.py#L3321-L3322

tm.assert_index_equal(df_index_0, exp_index_0)

expected = df.copy()
df = df.append({(MyEnum.A, 1): "abc", (MyEnum.B, 2): "xyz"},
ignore_index=True)
expected.loc[0, [(MyEnum.A, 1), (MyEnum.B, 2)]] = 'abc', 'xyz'
tm.assert_frame_equal(df, expected)