Skip to content

Commit b87fc41

Browse files
author
tp
committed
Fix bug where df.agg(..., axis=1) gives wrong result
1 parent bc37ea2 commit b87fc41

File tree

5 files changed

+174
-7
lines changed

5 files changed

+174
-7
lines changed

Diff for: ‎doc/source/whatsnew/v0.23.1.txt

+5
Original file line numberDiff line numberDiff line change
@@ -94,3 +94,8 @@ Categorical
9494
^^^^^^^^^^^
9595

9696
-
97+
98+
Numeric
99+
^^^^^^^
100+
101+
- :meth:`~DataFrame.agg` now handles built-in methods like ``sum`` in the same manner when axis=1 as when axis=0 (:issue:`21224`)

Diff for: ‎pandas/conftest.py

+17
Original file line numberDiff line numberDiff line change
@@ -149,3 +149,20 @@ def tz_aware_fixture(request):
149149
Fixture for trying explicit timezones: {0}
150150
"""
151151
return request.param
152+
153+
154+
@pytest.fixture(
155+
# params: Python 3.5 randomizes dict access and xdist doesn't like that
156+
# in fixtures. In order to get predetermined values we need to sort
157+
# the list deterministically
158+
# GH21224
159+
params=list(sorted(pd.core.base.SelectionMixin._cython_table.items(),
160+
key=lambda x: x[0].__name__)),
161+
ids=lambda x: "({}-{!r})_fixture".format(x[0].__name__, x[1]),
162+
)
163+
def cython_table_items(request):
164+
"""
165+
Fixture for returning the items in
166+
pandas.core.base.SelectionMixin._cython_table
167+
"""
168+
return request.param

Diff for: ‎pandas/core/frame.py

+9-6
Original file line numberDiff line numberDiff line change
@@ -5818,17 +5818,20 @@ def _gotitem(self,
58185818
def aggregate(self, func, axis=0, *args, **kwargs):
58195819
axis = self._get_axis_number(axis)
58205820

5821-
# TODO: flipped axis
58225821
result = None
5823-
if axis == 0:
5824-
try:
5825-
result, how = self._aggregate(func, axis=0, *args, **kwargs)
5826-
except TypeError:
5827-
pass
5822+
try:
5823+
result, how = self._aggregate(func, axis=axis, *args, **kwargs)
5824+
except TypeError:
5825+
pass
58285826
if result is None:
58295827
return self.apply(func, axis=axis, args=args, **kwargs)
58305828
return result
58315829

5830+
@Appender(NDFrame._aggregate.__doc__, indents=2)
5831+
def _aggregate(self, arg, axis=0, *args, **kwargs):
5832+
obj = self.T if axis == 1 else self
5833+
return super(DataFrame, obj)._aggregate(arg, *args, **kwargs)
5834+
58325835
agg = aggregate
58335836

58345837
def apply(self, func, axis=0, broadcast=None, raw=False, reduce=None,

Diff for: ‎pandas/tests/frame/test_apply.py

+71-1
Original file line numberDiff line numberDiff line change
@@ -946,7 +946,7 @@ def test_agg_dict_nested_renaming_depr(self):
946946
df = pd.DataFrame({'A': range(5), 'B': 5})
947947

948948
# nested renaming
949-
with tm.assert_produces_warning(FutureWarning):
949+
with tm.assert_produces_warning(FutureWarning, check_stacklevel=False):
950950
df.agg({'A': {'foo': 'min'},
951951
'B': {'bar': 'max'}})
952952

@@ -1056,3 +1056,73 @@ def test_non_callable_aggregates(self):
10561056
expected = df.size
10571057

10581058
assert result == expected
1059+
1060+
@pytest.mark.parametrize("frame, expected_dict", [
1061+
[DataFrame(), {
1062+
'sum': Series(),
1063+
'max': Series(),
1064+
'min': Series(),
1065+
'all': Series(dtype=bool),
1066+
'any': Series(dtype=bool),
1067+
'mean': Series(),
1068+
'prod': Series(),
1069+
'std': Series(),
1070+
'var': Series(),
1071+
'median': Series(),
1072+
'cumprod': DataFrame(),
1073+
'cumsum': DataFrame(),
1074+
}],
1075+
[DataFrame([[np.nan, 1], [1, 2]]), {
1076+
'sum': Series([1., 3]),
1077+
'max': Series([1., 2]),
1078+
'min': Series([1., 1]),
1079+
'all': Series([True, True]),
1080+
'any': Series([True, True]),
1081+
'mean': Series([1, 1.5]),
1082+
'prod': Series([1., 2]),
1083+
'std': Series([np.nan, 0.707107]),
1084+
'var': Series([np.nan, 0.5]),
1085+
'median': Series([1, 1.5]),
1086+
'cumprod': DataFrame([[np.nan, 1], [1., 2.]]),
1087+
'cumsum': DataFrame([[np.nan, 1], [1., 3.]]),
1088+
}],
1089+
[DataFrame([['a', 'b'], ['b', 'a']]), {
1090+
'sum': Series(['ab', 'ba']),
1091+
'max': Series(['b', 'b']),
1092+
'min': Series(['a', 'a']),
1093+
'all': Series([True, True]),
1094+
'any': Series([True, True]),
1095+
'mean': Series([], index=pd.Index([], dtype='int64')),
1096+
'prod': Series([], index=pd.Index([], dtype='int64')),
1097+
'std': Series([], index=pd.Index([], dtype='int64')),
1098+
'var': Series([], index=pd.Index([], dtype='int64')),
1099+
'median': Series([], index=pd.Index([], dtype='int64')),
1100+
'cumprod': TypeError,
1101+
'cumsum': DataFrame([['a', 'b'], ['ab', 'ba']]),
1102+
}],
1103+
])
1104+
@pytest.mark.parametrize("axis", [0, 1], ids=lambda x: "axis {}".format(x))
1105+
def test_agg_cython_table(self, cython_table_items,
1106+
frame, expected_dict, axis):
1107+
# GH21224
1108+
# test if using items in pandas.core.base.SelectionMixin._cython_table
1109+
# in agg gives correct results
1110+
np_func, str_func = cython_table_items
1111+
expected = expected_dict[str_func]
1112+
1113+
if isinstance(expected, type) and issubclass(expected, Exception):
1114+
with pytest.raises(expected):
1115+
# e.g. DataFrame(['a b'.split()]).cumprod() will raise
1116+
frame.agg(np_func, axis=axis)
1117+
with pytest.raises(expected):
1118+
frame.agg(str_func, axis=axis)
1119+
return
1120+
1121+
result = frame.agg(np_func, axis=axis)
1122+
result_str_func = frame.agg(str_func, axis=axis)
1123+
if str_func in ('cumprod', 'cumsum'):
1124+
tm.assert_frame_equal(result, expected)
1125+
tm.assert_frame_equal(result_str_func, expected)
1126+
else:
1127+
tm.assert_series_equal(result, expected)
1128+
tm.assert_series_equal(result_str_func, expected)

Diff for: ‎pandas/tests/series/test_apply.py

+72
Original file line numberDiff line numberDiff line change
@@ -331,6 +331,78 @@ def test_non_callable_aggregates(self):
331331
('mean', 1.5)]))
332332
assert_series_equal(result[expected.index], expected)
333333

334+
@pytest.mark.parametrize("series, expected_dict", [
335+
[Series(), {
336+
'sum': 0,
337+
'max': np.nan,
338+
'min': np.nan,
339+
'all': True,
340+
'any': False,
341+
'mean': np.nan,
342+
'prod': 1,
343+
'std': np.nan,
344+
'var': np.nan,
345+
'median': np.nan,
346+
'cumprod': Series([], Index([])),
347+
'cumsum': Series([], Index([])),
348+
}],
349+
[Series([np.nan, 1, 2, 3]), {
350+
'sum': 6,
351+
'max': 3,
352+
'min': 1,
353+
'all': True,
354+
'any': True,
355+
'mean': 2,
356+
'prod': 6,
357+
'std': 1,
358+
'var': 1,
359+
'median': 2,
360+
'cumprod': Series([np.nan, 1, 2, 6]),
361+
'cumsum': Series([np.nan, 1, 3, 6]),
362+
}],
363+
[Series('a b c'.split()), {
364+
'sum': 'abc',
365+
'max': 'c',
366+
'min': 'a',
367+
'all': 'c', # see GH12863
368+
'any': 'a',
369+
'mean': TypeError, # mean raises TypeError
370+
'prod': TypeError,
371+
'std': TypeError,
372+
'var': TypeError,
373+
'median': TypeError,
374+
'cumprod': TypeError,
375+
'cumsum': Series(['a', 'ab', 'abc']),
376+
}],
377+
])
378+
def test_agg_cython_table(self, cython_table_items,
379+
series, expected_dict):
380+
# GH21224
381+
# test if using items in pandas.core.base.SelectionMixin._cython_table
382+
# in agg gives correct results
383+
np_func, str_func = cython_table_items
384+
expected = expected_dict[str_func]
385+
386+
if isinstance(expected, type) and issubclass(expected, Exception):
387+
with pytest.raises(expected):
388+
# e.g. Series('a b'.split()).cumprod() will raise
389+
series.agg(np_func)
390+
with pytest.raises(expected):
391+
series.agg(str_func)
392+
return
393+
394+
result = series.agg(np_func)
395+
result_str_func = series.agg(str_func)
396+
if str_func in ('cumprod', 'cumsum'):
397+
tm.assert_series_equal(result, expected)
398+
tm.assert_series_equal(result_str_func, expected)
399+
elif tm.is_number(expected):
400+
assert np.isclose(result, expected, equal_nan=True)
401+
assert np.isclose(result_str_func, expected, equal_nan=True)
402+
else:
403+
assert result == expected
404+
assert result_str_func == expected
405+
334406

335407
class TestSeriesMap(TestData):
336408

0 commit comments

Comments
 (0)