Prevent UnicodeDecodeError in pivot_table under Py2 #17489

mpenkov · 2017-09-10T07:33:05Z

closes Exception when adding a unicode margins_name in pivot_table #13292
tests added / passed
passes git diff upstream/master -u -- "*.py" | flake8 --diff
whatsnew entry

gfyoung · 2017-09-10T07:36:26Z

@mpenkov : Will need a whatsnew entry

codecov · 2017-09-10T08:29:24Z

Codecov Report

Merging #17489 into master will decrease coverage by 0.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master   #17489      +/-   ##
==========================================
- Coverage   91.15%   91.14%   -0.02%     
==========================================
  Files         163      163              
  Lines       49534    49534              
==========================================
- Hits        45153    45148       -5     
- Misses       4381     4386       +5

Flag	Coverage Δ
#multiple	`88.92% <100%> (ø)`	⬆️
#single	`40.22% <50%> (-0.07%)`	⬇️

Impacted Files	Coverage Δ
pandas/core/reshape/pivot.py	`96.35% <100%> (+0.99%)`	⬆️
pandas/io/gbq.py	`25% <0%> (-58.34%)`	⬇️
pandas/core/frame.py	`97.77% <0%> (-0.05%)`	⬇️

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 23050dc...e851082. Read the comment docs.

codecov · 2017-09-10T08:29:26Z

Codecov Report

Merging #17489 into master will decrease coverage by 0.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master   #17489      +/-   ##
==========================================
- Coverage   91.15%   91.14%   -0.02%     
==========================================
  Files         163      163              
  Lines       49534    49534              
==========================================
- Hits        45153    45148       -5     
- Misses       4381     4386       +5

Flag	Coverage Δ
#multiple	`88.92% <100%> (ø)`	⬆️
#single	`40.22% <0%> (-0.07%)`	⬇️

Impacted Files	Coverage Δ
pandas/core/reshape/pivot.py	`96.35% <100%> (+0.99%)`	⬆️
pandas/io/gbq.py	`25% <0%> (-58.34%)`	⬇️
pandas/core/frame.py	`97.77% <0%> (-0.05%)`	⬇️
pandas/io/feather_format.py	`85.71% <0%> (ø)`	⬆️

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 23050dc...ce96abd. Read the comment docs.

mpenkov · 2017-09-10T12:06:03Z

@gfyoung Added the what's new. Please check.

TomAugspurger · 2017-09-10T13:19:33Z

pandas/core/reshape/pivot.py

@@ -145,7 +145,7 @@ def _add_margins(table, data, values, rows, cols, aggfunc,
    if not isinstance(margins_name, compat.string_types):
        raise ValueError('margins_name argument must be a string')

-    msg = 'Conflicting name "{name}" in margins'.format(name=margins_name)
+    msg = u('Conflicting name "{name}" in margins').format(name=margins_name)


The new parentheses can be removed.

jreback · 2017-09-10T14:17:34Z

pandas/tests/reshape/test_pivot.py

+    def test_issue_13292(self):
+        # The below shouldn't raise an exception anymore.
+        frame = pd.DataFrame({'foo': [1, 2, 3]})
+        pd.pivot_table(frame, index=['foo'], aggfunc=len, margins=True,


you don't need the comment, instead change the test name to something useful like: test_pivot_margins_name_unicode and put a comment with the issue number

jreback · 2017-09-10T14:17:59Z

pandas/tests/reshape/test_pivot.py

+        # The below shouldn't raise an exception anymore.
+        frame = pd.DataFrame({'foo': [1, 2, 3]})
+        pd.pivot_table(frame, index=['foo'], aggfunc=len, margins=True,
+                       margins_name=u'\u0394\u03bf\u03ba\u03b9\u03bc\u03ae')


get the result = pd.pivot_table(....)

and compare to a constructed expected with tm.assert_frame_equal

@jreback Could you please suggest what the constructed expected should look like? I'm a bit new and unsure of what should go there. Here's what I've got so far:

def test_pivot_margins_name_unicode(self): # issue #13292 greek = u'\u0394\u03bf\u03ba\u03b9\u03bc\u03ae' frame = pd.DataFrame({'foo': [1, 2, 3]}) table = pd.pivot_table(frame, index=['foo'], aggfunc=len, margins=True, margins_name=greek) expected = pd.DataFrame({}, columns=['foo'], index=[1, 2, 3, greek]) tm.assert_frame_equal(table, expected)

This is failing on the last line with DataFrame shape mismatch.

don't pass the {}

Thanks for the suggestion. This is what I have now:

expected = pd.DataFrame(columns=['foo'], index=[1, 2, 3, greek])

It's still failing:

E AssertionError: DataFrame are different E E DataFrame shape mismatch E [left]: (4, 0) E [right]: (4, 1) pandas/util/testing.py:1105: AssertionError

What else could be wrong?

jreback · 2017-09-10T14:18:57Z

doc/source/whatsnew/v0.20.0.txt

@@ -1705,6 +1705,7 @@ Reshaping
 - Bug in ``pd.concat()`` in which concatenating with an empty dataframe with ``join='inner'`` was being improperly handled (:issue:`15328`)
 - Bug with ``sort=True`` in ``DataFrame.join`` and ``pd.merge`` when joining on indexes (:issue:`15582`)
 - Bug in ``DataFrame.nsmallest`` and ``DataFrame.nlargest`` where identical values resulted in duplicated rows (:issue:`15297`)
+- Bug in ``pivot._add_margins` when ``margins_name`` is Unicode under Py2 (:issue:`13292`)


write this from a user perspective

Bug in :func:`pandas.pivot_table` incorrectly raising when passing unicode input for margins keyword

jreback · 2017-09-10T20:27:32Z

pandas/tests/reshape/test_pivot.py

+    def test_pivot_margins_name_unicode(self):
+        # issue #13292
+        frame = pd.DataFrame({'foo': [1, 2, 3]})
+        pd.pivot_table(frame, index=['foo'], aggfunc=len, margins=True,


still like to compare this here

I figured out how to construct the expected object correctly. Please have a look.

jreback · 2017-09-12T10:31:36Z

thanks @mpenkov

mpenkov added 2 commits September 10, 2017 15:18

resolve issue #13292

45e99bf

dummy commit to trigger travis build on my branch

01991e9

gfyoung added Error Reporting Incorrect or improved errors from pandas Unicode Unicode strings labels Sep 10, 2017

update whatsnew

e851082

TomAugspurger reviewed Sep 10, 2017

View reviewed changes

jreback requested changes Sep 10, 2017

View reviewed changes

responding to (most) review comments

0d593cb

jreback requested changes Sep 10, 2017

View reviewed changes

added assertion to test_pivot_margins_name_unicode

ce96abd

jreback added this to the 0.21.0 milestone Sep 12, 2017

jreback approved these changes Sep 12, 2017

View reviewed changes

jreback merged commit d46b027 into pandas-dev:master Sep 12, 2017

alanbato pushed a commit to alanbato/pandas that referenced this pull request Nov 10, 2017

Prevent UnicodeDecodeError in pivot_table under Py2 (pandas-dev#17489)

bada33d

No-Stream pushed a commit to No-Stream/pandas that referenced this pull request Nov 28, 2017

Prevent UnicodeDecodeError in pivot_table under Py2 (pandas-dev#17489)

0c6eef9

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Prevent UnicodeDecodeError in pivot_table under Py2 #17489

Prevent UnicodeDecodeError in pivot_table under Py2 #17489

mpenkov commented Sep 10, 2017 •

edited

Loading

gfyoung commented Sep 10, 2017

codecov bot commented Sep 10, 2017

codecov bot commented Sep 10, 2017 •

edited

Loading

mpenkov commented Sep 10, 2017

TomAugspurger Sep 10, 2017

jreback Sep 10, 2017

jreback Sep 10, 2017

mpenkov Sep 10, 2017 •

edited

Loading

jreback Sep 10, 2017

mpenkov Sep 10, 2017

jreback Sep 10, 2017

jreback Sep 10, 2017

mpenkov Sep 12, 2017

jreback commented Sep 12, 2017

Prevent UnicodeDecodeError in pivot_table under Py2 #17489

Prevent UnicodeDecodeError in pivot_table under Py2 #17489

Conversation

mpenkov commented Sep 10, 2017 • edited Loading

gfyoung commented Sep 10, 2017

codecov bot commented Sep 10, 2017

Codecov Report

codecov bot commented Sep 10, 2017 • edited Loading

Codecov Report

mpenkov commented Sep 10, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mpenkov Sep 10, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jreback commented Sep 12, 2017

mpenkov commented Sep 10, 2017 •

edited

Loading

codecov bot commented Sep 10, 2017 •

edited

Loading

mpenkov Sep 10, 2017 •

edited

Loading