From 45e99bf36d8df30d49aea24ee8b83263c2618ae7 Mon Sep 17 00:00:00 2001 From: Michael Penkov Date: Sun, 10 Sep 2017 15:18:19 +0900 Subject: [PATCH 1/5] resolve issue #13292 --- pandas/core/reshape/pivot.py | 4 ++-- pandas/tests/reshape/test_pivot.py | 8 ++++++++ 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/pandas/core/reshape/pivot.py b/pandas/core/reshape/pivot.py index f07123ca18489..5873a0556daee 100644 --- a/pandas/core/reshape/pivot.py +++ b/pandas/core/reshape/pivot.py @@ -9,7 +9,7 @@ from pandas.core.groupby import Grouper from pandas.core.reshape.util import cartesian_product from pandas.core.index import Index, _get_objs_combined_axis -from pandas.compat import range, lrange, zip +from pandas.compat import range, lrange, zip, u from pandas import compat import pandas.core.common as com from pandas.util._decorators import Appender, Substitution @@ -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) for level in table.index.names: if margins_name in table.index.get_level_values(level): raise ValueError(msg) diff --git a/pandas/tests/reshape/test_pivot.py b/pandas/tests/reshape/test_pivot.py index 879ac96680fbb..a6faa2113bd11 100644 --- a/pandas/tests/reshape/test_pivot.py +++ b/pandas/tests/reshape/test_pivot.py @@ -1625,3 +1625,11 @@ def test_isleapyear_deprecate(self): with tm.assert_produces_warning(FutureWarning, check_stacklevel=False): assert isleapyear(2004) + + 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, + margins_name=u'\u0394\u03bf\u03ba\u03b9\u03bc\u03ae') From 01991e9ccc5b33f2c57da6088a8cfa581a9e97e5 Mon Sep 17 00:00:00 2001 From: Michael Penkov Date: Sun, 10 Sep 2017 15:31:32 +0900 Subject: [PATCH 2/5] dummy commit to trigger travis build on my branch --- pandas/tests/reshape/test_pivot.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/pandas/tests/reshape/test_pivot.py b/pandas/tests/reshape/test_pivot.py index a6faa2113bd11..d7ccba6ca2017 100644 --- a/pandas/tests/reshape/test_pivot.py +++ b/pandas/tests/reshape/test_pivot.py @@ -1627,9 +1627,7 @@ def test_isleapyear_deprecate(self): assert isleapyear(2004) 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, margins_name=u'\u0394\u03bf\u03ba\u03b9\u03bc\u03ae') From e8510820be0ef86235676086d0b8f95227ce19be Mon Sep 17 00:00:00 2001 From: Michael Penkov Date: Sun, 10 Sep 2017 17:28:56 +0900 Subject: [PATCH 3/5] update whatsnew --- doc/source/whatsnew/v0.20.0.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/doc/source/whatsnew/v0.20.0.txt b/doc/source/whatsnew/v0.20.0.txt index 9d475390175b2..26e159a7e6769 100644 --- a/doc/source/whatsnew/v0.20.0.txt +++ b/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`) Numeric ^^^^^^^ From 0d593cbc039a0b74a54d6fb268cfd7568be5111d Mon Sep 17 00:00:00 2001 From: Michael Penkov Date: Sun, 10 Sep 2017 23:46:26 +0900 Subject: [PATCH 4/5] responding to (most) review comments --- doc/source/whatsnew/v0.20.0.txt | 2 +- pandas/core/reshape/pivot.py | 4 ++-- pandas/tests/reshape/test_pivot.py | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/doc/source/whatsnew/v0.20.0.txt b/doc/source/whatsnew/v0.20.0.txt index 26e159a7e6769..fe24f8f499172 100644 --- a/doc/source/whatsnew/v0.20.0.txt +++ b/doc/source/whatsnew/v0.20.0.txt @@ -1705,7 +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`) +- Bug in :func:`pandas.pivot_table` incorrectly raising ``UnicodeError`` when passing unicode input for ```margins`` keyword (:issue:`13292`) Numeric ^^^^^^^ diff --git a/pandas/core/reshape/pivot.py b/pandas/core/reshape/pivot.py index 5873a0556daee..d19de6030d473 100644 --- a/pandas/core/reshape/pivot.py +++ b/pandas/core/reshape/pivot.py @@ -9,7 +9,7 @@ from pandas.core.groupby import Grouper from pandas.core.reshape.util import cartesian_product from pandas.core.index import Index, _get_objs_combined_axis -from pandas.compat import range, lrange, zip, u +from pandas.compat import range, lrange, zip from pandas import compat import pandas.core.common as com from pandas.util._decorators import Appender, Substitution @@ -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 = u('Conflicting name "{name}" in margins').format(name=margins_name) + msg = u'Conflicting name "{name}" in margins'.format(name=margins_name) for level in table.index.names: if margins_name in table.index.get_level_values(level): raise ValueError(msg) diff --git a/pandas/tests/reshape/test_pivot.py b/pandas/tests/reshape/test_pivot.py index d7ccba6ca2017..53f610a3de586 100644 --- a/pandas/tests/reshape/test_pivot.py +++ b/pandas/tests/reshape/test_pivot.py @@ -1626,8 +1626,8 @@ def test_isleapyear_deprecate(self): with tm.assert_produces_warning(FutureWarning, check_stacklevel=False): assert isleapyear(2004) - def test_issue_13292(self): - # The below shouldn't raise an exception anymore. + 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, margins_name=u'\u0394\u03bf\u03ba\u03b9\u03bc\u03ae') From ce96abdf173ce88e0791ef02092f47f1ac1fa768 Mon Sep 17 00:00:00 2001 From: Michael Penkov Date: Tue, 12 Sep 2017 12:51:16 +0900 Subject: [PATCH 5/5] added assertion to test_pivot_margins_name_unicode --- pandas/tests/reshape/test_pivot.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/pandas/tests/reshape/test_pivot.py b/pandas/tests/reshape/test_pivot.py index 53f610a3de586..bd8a999ce2330 100644 --- a/pandas/tests/reshape/test_pivot.py +++ b/pandas/tests/reshape/test_pivot.py @@ -1628,6 +1628,10 @@ def test_isleapyear_deprecate(self): def test_pivot_margins_name_unicode(self): # issue #13292 + greek = u'\u0394\u03bf\u03ba\u03b9\u03bc\u03ae' 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') + table = pd.pivot_table(frame, index=['foo'], aggfunc=len, margins=True, + margins_name=greek) + index = pd.Index([1, 2, 3, greek], dtype='object', name='foo') + expected = pd.DataFrame(index=index) + tm.assert_frame_equal(table, expected)