Skip to content

BUG: Fix for GH #14848 for groupby().describe() with tuples as the Index #15110

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 5 commits into from
Closed

BUG: Fix for GH #14848 for groupby().describe() with tuples as the Index #15110

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
9489cb2
BUG: Fix issue #14848 groupby().describe() on indices containing all …
Dr-Irv Jan 12, 2017
f3a7a21
Changes per jreback requests
Dr-Irv Jan 12, 2017
fbd20f5
Raise error when creating index of tuples with name parameter a string
Dr-Irv Jan 12, 2017
db13c3b
Use not is_list_like
Dr-Irv Jan 12, 2017
c18c6cb
Undo change to merge.py and make whatsnew a 2 line comment.
Dr-Irv Jan 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
1 change: 1 addition & 0 deletions doc/source/whatsnew/v0.20.0.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -351,6 +351,7 @@ Bug Fixes
- Bug in converting object elements of array-like objects to unsigned 64-bit integers (:issue:`4471`, :issue:`14982`)
- Bug in ``pd.pivot_table()`` where no error was raised when values argument was not in the columns (:issue:`14938`)

- Bug in ``DataFrame.groupby().describe()`` when grouping on ``Index`` containing tuples (:issue:`14848`)



Expand Down
6 changes: 6 additions & 0 deletions pandas/indexes/multi.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
is_object_dtype,
is_iterator,
is_list_like,
is_string_like,
is_scalar)
from pandas.types.missing import isnull, array_equivalent
from pandas.core.common import (_values_from_object,
Expand Down Expand Up @@ -490,6 +491,11 @@ def _set_names(self, names, level=None, validate=True):
that it only acts on copies
"""

# GH 15110
# Don't allow a single string for names in a MultiIndex
if names is not None and is_string_like(names):
Copy link
Contributor

Choose a reason for hiding this comment

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

I would use not is_list_like here (which excludes things like TImestamps and strings), but allows other list-like things

raise ValueError('Names should not be a single string for a '
'MultiIndex.')
names = list(names)

if validate and level is not None and len(names) != len(level):
Expand Down
13 changes: 13 additions & 0 deletions pandas/tests/groupby/test_groupby.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1490,6 +1490,19 @@ def test_frame_describe_multikey(self):
for name, group in groupedT:
assert_frame_equal(result[name], group.describe())

def test_frame_describe_tupleindex(self):

# GH 14848 - regression from 0.19.0 to 0.19.1
df1 = DataFrame({'x': [1, 2, 3, 4, 5] * 3,
Copy link
Contributor

Choose a reason for hiding this comment

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

comment inside the test

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK

'y': [10, 20, 30, 40, 50] * 3,
'z': [100, 200, 300, 400, 500] * 3})
df1['k'] = [(0, 0, 1), (0, 1, 0), (1, 0, 0)] * 5
df2 = df1.rename(columns={'k': 'key'})
result = df1.groupby('k').describe()
expected = df2.groupby('key').describe()
expected.index.set_names(result.index.names, inplace=True)
assert_frame_equal(result, expected)

def test_frame_groupby(self):
grouped = self.tsframe.groupby(lambda x: x.weekday())

Expand Down
9 changes: 9 additions & 0 deletions pandas/tests/indexes/test_multi.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2554,3 +2554,12 @@ def test_unsortedindex(self):

with assertRaises(KeyError):
df.loc(axis=0)['q', :]

def test_tuples_with_name_string(self):
# GH 15110 and GH 14848

li = [(0, 0, 1), (0, 1, 0), (1, 0, 0)]
with assertRaises(ValueError):
pd.Index(li, name='abc')
with assertRaises(ValueError):
pd.Index(li, name='a')
6 changes: 5 additions & 1 deletion pandas/tools/merge.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1626,7 +1626,11 @@ def __init__(self, objs, axis=0, join='outer', join_axes=None,
clean_objs.append(v)
objs = clean_objs
name = getattr(keys, 'name', None)
keys = Index(clean_keys, name=name)
# GH 14848
Copy link
Contributor

Choose a reason for hiding this comment

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

why do we need the check here? the Index already has a fast path for this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jreback Because in the original issue, if keys is an Index containing all tuples, and clean_keys then becomes a list of tuples, and name is a single string, that's where things go wrong. What the fix does is avoid creating an Index (and let the name propagate) because the Index has already been created.

Copy link
Contributor

Choose a reason for hiding this comment

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

hmm

maybe this should be handled in the _get_grouper logic then

where you first figure out what the name actually is

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jreback It's not a grouper issue. Here's code that fails with 0.19.2 when passing an Index of tuples into concat.

df1 = pd.DataFrame({"x" : [1,2,3], "k" : [(0,0,1),(0,1,0),(1,0,0)]}).set_index('k')
df2 = pd.DataFrame({"x" : [4,5,6]}, index = pd.Index(['a','b','c']))
ic2 = df1.index
ic1 = pd.Index(['g','h','i'], name = 'foobar')
c1 = pd.concat([df2, df2, df2], axis = 0, keys=ic1, levels=[ic1], names=[ic1.name])
print('c1 is ok')
c2 = pd.concat([df2, df2, df2], axis = 0, keys=ic2, levels=[ic2], names=[ic2.name])
print('c2 is ok')

The value of c1 is computed fine. The value of c2 raises an error because the tuples were passed as the index into concat. But if you use the code that I put in above in merge.py, then this example works.

I know the example is a bit far-fetched, but it is equivalent to what is going on when the grouper calls concat, and I'd rather not mess with the grouper when the simple fix handles this particular bug.

Copy link
Contributor

Choose a reason for hiding this comment

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

In [1]: li = [(0, 0, 1), (0, 1, 0), (1, 0, 0)]

In [2]: pd.Index(li)
Out[2]: 
MultiIndex(levels=[[0, 1], [0, 1], [0, 1]],
           labels=[[0, 0, 1], [0, 1, 0], [1, 0, 0]])

In [3]: pd.Index(li, name='foo')
Out[3]: 
MultiIndex(levels=[[0, 1], [0, 1], [0, 1]],
           labels=[[0, 0, 1], [0, 1, 0], [1, 0, 0]],
           names=['f', 'o', 'o'])

this is with this PR, something wrong

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jreback Looks like I didn't push the last set of changes. Sorry about that.

Copy link
Contributor

Choose a reason for hiding this comment

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

so I remove the change in this file and everything still works. Please do that and push up again, the only fix here needed was in MultiIndex.

# If you already have an Index, no need
# to recreate it
if not isinstance(keys, Index):
keys = Index(clean_keys, name=name)

if len(objs) == 0:
raise ValueError('All objects passed were None')
Expand Down