BUG fixes tuple agg issue 18079 #18354

bobhaffner · 2017-11-18T04:21:32Z

closes DataFrameGroupBy.aggregate can not work with tuple as an argument #18079
tests added / passed
passes git diff upstream/master -u -- "*.py" | flake8 --diff
whatsnew entry

codecov · 2017-11-18T04:52:45Z

Codecov Report

Merging #18354 into master will not change coverage.
The diff coverage is 100%.

@@           Coverage Diff           @@
##           master   #18354   +/-   ##
=======================================
  Coverage   91.33%   91.33%           
=======================================
  Files         163      163           
  Lines       49801    49801           
=======================================
  Hits        45487    45487           
  Misses       4314     4314

Flag	Coverage Δ
#multiple	`89.13% <100%> (ø)`	⬆️
#single	`40.79% <0%> (+0.02%)`	⬆️

Impacted Files	Coverage Δ
pandas/core/groupby.py	`92.03% <100%> (ø)`	⬆️

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 d101064...7a4342f. Read the comment docs.

TomAugspurger · 2017-11-18T13:00:34Z

doc/source/whatsnew/v0.21.1.txt

@@ -62,6 +62,7 @@ Bug Fixes
 - Bug in ``pd.Series.rolling.skew()`` and ``rolling.kurt()`` with all equal values has floating issue (:issue:`18044`)
 - Bug in ``pd.DataFrameGroupBy.count()`` when counting over a datetimelike column (:issue:`13393`)
 - Bug in ``pd.concat`` when empty and non-empty DataFrames or Series are concatenated (:issue:`18178` :issue:`18187`)
+- Bug in :class:`NDFrameGroupBy` fixes ValueError: no results error when grouping by a single column and aggregating with a tuple (:issue:`18079`)  


Maybe just say something like "Bug when grouping by a single column and aggregating with a class like list or tuple" . I don't recall if NDFRameGroupBy is in the public API.

TomAugspurger · 2017-11-18T13:01:37Z

pandas/tests/groupby/test_groupby.py

+        assert_frame_equal(result, expected)
+
+        result = df.groupby('A')['C'].aggregate(tuple)
+        expected = pd.Series([(1, 1, 1), (3, 4, 4)], index=[1, 3])


Should be able to pass name='C' here.

bobhaffner · 2017-11-18T14:41:57Z

@TomAugspurger's comment on whats new prompted me to add a test for list aggregation only to discover a new issue.

This works
df.groupby('A')['C'].aggregate(list)

This doesn't.
df.groupby(['A', 'B']).aggregate(list)

Should I submit a separate issue for this or investigate and fix this new error as part of this PR?

Click to see full traceback

---------------------------------------------------------------------------

Traceback (most recent call last):
File "/Users/bob/dev/pandas-dev/pandas/core/groupby.py", line 2267, in agg_series
return self._aggregate_series_fast(obj, func)
File "/Users/bob/dev/pandas-dev/pandas/core/groupby.py", line 2287, in _aggregate_series_fast
result, counts = grouper.get_result()
File "pandas/_libs/src/reduce.pyx", line 406, in pandas._libs.lib.SeriesGrouper.get_result
raise
File "pandas/_libs/src/reduce.pyx", line 394, in pandas._libs.lib.SeriesGrouper.get_result
result = _get_result_array(res,
File "pandas/_libs/src/reduce.pyx", line 15, in pandas._libs.lib._get_result_array
raise ValueError('function does not reduce')
ValueError: function does not reduce

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
File "", line 1, in
File "/Users/bob/dev/pandas-dev/pandas/core/groupby.py", line 4192, in aggregate
return super(DataFrameGroupBy, self).aggregate(arg, *args, **kwargs)
File "/Users/bob/dev/pandas-dev/pandas/core/groupby.py", line 3634, in aggregate
return self._python_agg_general(arg, *args, **kwargs)
File "/Users/bob/dev/pandas-dev/pandas/core/groupby.py", line 853, in _python_agg_general
result, counts = self.grouper.agg_series(obj, f)
File "/Users/bob/dev/pandas-dev/pandas/core/groupby.py", line 2269, in agg_series
return self._aggregate_series_pure_python(obj, func)
File "/Users/bob/dev/pandas-dev/pandas/core/groupby.py", line 2304, in _aggregate_series_pure_python
raise ValueError('Function does not reduce')
ValueError: Function does not reduce

jreback · 2017-11-19T15:54:54Z

pandas/tests/groupby/test_groupby.py

+        # Issue #18079
+        df = pd.DataFrame({'A': [1, 1, 1, 3, 3, 3],
+                           'B': [1, 1, 1, 4, 4, 4], 'C': [1, 1, 1, 3, 4, 4]})
+


move to test_aggregate

can you parametrize with list-likes here (e.g. tuple/list,np.array,Series)

some of these might have different output (Series) but let's see.

you may have to have a tests with the parameterized ones and another one with 'other' things, e.g.

def f(x): return tuple(x)

which work now.

Thanks for the feedback, @jreback! I'm hoping to tackle the list bug and these additional tests later this week.

bobhaffner · 2017-11-25T19:29:32Z

Moved tests to test_aggregate.py
Parametrize the tests (one for DataFrames and one for Series))
I fixed the list issue I mentioned above by removing the list condition in the following if

pandas/pandas/core/groupby.py

Line 2303 in 4fce784

isinstance(res, list)):

I was a little leery doing this, but all tests passed.

Note: Both np.array and pd.Series still error out with both scenarios (grouping by one column vs multiple columns), but with different error descriptions. That said, is it possible to proceed with this PR since both tuple and list now behave the same way with single and multiple column groupbys?

df = pd.DataFrame({'A' : [1, 1, 3], 'B' :  [1, 1, 4], 'C' :  [1, 3, 4]})

df.groupby('A')['C'].aggregate(pd.Series) # np.array
>> Exception: Must produce aggregated value

df.groupby(['A', 'B']).aggregate(pd.Series) # np.array
>> ValueError: Function does not reduce

This might close #4293 as well

jreback

small changes, otherwise lgtm. ping on green.

jreback · 2017-11-25T21:26:53Z

doc/source/whatsnew/v0.21.1.txt

@@ -103,7 +103,7 @@ Groupby/Resample/Rolling
 - Bug in ``DataFrame.resample(...).apply(...)`` when there is a callable that returns different columns (:issue:`15169`)
 - Bug in ``DataFrame.resample(...)`` when there is a time change (DST) and resampling frequecy is 12h or higher (:issue:`15549`)
 - Bug in ``pd.DataFrameGroupBy.count()`` when counting over a datetimelike column (:issue:`13393`)
-
+- Bug when grouping by a single column and aggregating with a class like`list` or `tuple` (:issue:`18079`)


use double-backticks around list and tuple

move this to 0.22 as well. (its not that I don't regard this as a bug fix, but this is something touching an important piece of code and want this to live in master for a bit).

bobhaffner · 2017-11-25T22:48:12Z

Thanks @jreback!

jreback · 2017-11-25T23:09:55Z

one more rebase :>

pep8speaks · 2017-11-25T23:54:54Z

Hello @bobhaffner! Thanks for updating the PR.

Cheers ! There are no PEP8 issues in this Pull Request. 🍻

Comment last updated on November 26, 2017 at 16:18 Hours UTC

bobhaffner · 2017-11-26T00:05:06Z

Hi @jreback I had conflicts (whatsnew) with the rebase, but I believe Its corrected now. That said, I'm puzzled by the pep8speaks comment regarding test_agg.py though as I didn't modify that file. My apologies for the confusion.

jreback

looks like some extra files got added during the rebase. pls remove them. and push again.

jreback · 2017-11-26T00:09:26Z

doc/source/whatsnew/v0.21.1.txt.orig

+
+.. _whatsnew_0211.bug_fixes:
+
+Bug Fixes


remove this

jreback · 2017-11-26T00:09:33Z

grp.patch

+         if isinstance(func_or_funcs, compat.string_types):
+             return getattr(self, func_or_funcs)(*args, **kwargs)
+
+-        if hasattr(func_or_funcs, '__iter__'):


remove this

jreback · 2017-11-26T00:09:43Z

test_agg.py

+   return list(x)
+
+#df = pd.DataFrame({'A' : [1, 1, 3], 'B' :  [1, 2, 4]})
+#result = df.groupby('A').aggregate(f)


remove this

bobhaffner · 2017-11-26T00:14:02Z

Yikes, my mistake. thanks Jeff.

jreback

couple of more items to remove

jreback · 2017-11-26T15:13:58Z

grp_test.patch

+@@ -2725,3 +2725,12 @@ def _check_groupby(df, result, keys, field, f=lambda x: x.sum()):
+     expected = f(df.groupby(tups)[field])
+     for k, v in compat.iteritems(expected):
+         assert (result[k] == v)


patch needs removal

jreback · 2017-11-26T15:14:05Z

doc/source/whatsnew/v0.21.1.txt

@@ -147,4 +147,4 @@ Other
 ^^^^^

 -


can you revert this file

HI Jeff, I'm afraid my lack of git experience is showing as this file has been a bit of a thorn in my side. I'm concerned that my attempts to fix my mess will just result in wasting more of your time. Would you mind sharing the syntax necessary to revert (or reset?) this file to the proper commit?

sure ill fix this upl

jreback · 2017-11-26T16:19:14Z

should be good to go. ping on green.

bobhaffner · 2017-11-26T16:26:39Z

Thanks Jeff! You're a lifesaver!

jreback · 2017-11-26T23:42:57Z

thanks @bobhaffner

bobhaffner · 2017-11-27T00:15:43Z

Many thanks for the help and the patience, @jreback . Same to you @TomAugspurger

TomAugspurger reviewed Nov 18, 2017

View reviewed changes

jreback requested changes Nov 19, 2017

View reviewed changes

jreback added Compat pandas objects compatability with Numpy or Python functions Groupby Reshaping Concat, Merge/Join, Stack/Unstack, Explode Bug and removed Compat pandas objects compatability with Numpy or Python functions labels Nov 19, 2017

bobhaffner force-pushed the grp_tuple_bug_fix branch from dc18825 to 5fa71e1 Compare November 25, 2017 19:07

jreback requested changes Nov 25, 2017

View reviewed changes

bobhaffner added 4 commits November 25, 2017 17:24

fix list agg and new tests

bfeaf55

added whatsnew

836ef4c

rebase cleanup

374c370

backticks and moved to .22

6639d48

bobhaffner force-pushed the grp_tuple_bug_fix branch from 2be5881 to 6639d48 Compare November 25, 2017 23:54

jreback requested changes Nov 26, 2017

View reviewed changes

removed local files

1361f74

jreback requested changes Nov 26, 2017

View reviewed changes

jreback added 2 commits November 26, 2017 11:16

Merge branch 'master' into PR_TOOL_MERGE_PR_18354

7390591

fixups

7a4342f

jreback added this to the 0.22.0 milestone Nov 26, 2017

jreback approved these changes Nov 26, 2017

View reviewed changes

jreback merged commit 674fb96 into pandas-dev:master Nov 26, 2017

bobhaffner deleted the grp_tuple_bug_fix branch November 27, 2017 00:16

toobaz mentioned this pull request May 18, 2018

"Function does not reduce" for multiindex, but works fine for single index. #4293

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

BUG fixes tuple agg issue 18079 #18354

BUG fixes tuple agg issue 18079 #18354

bobhaffner commented Nov 18, 2017 •

edited

Loading

codecov bot commented Nov 18, 2017 •

edited

Loading

TomAugspurger Nov 18, 2017

TomAugspurger Nov 18, 2017

bobhaffner commented Nov 18, 2017

jreback Nov 19, 2017

bobhaffner Nov 21, 2017

bobhaffner commented Nov 25, 2017

jreback left a comment

jreback Nov 25, 2017

jreback Nov 25, 2017

bobhaffner commented Nov 25, 2017

jreback commented Nov 25, 2017

pep8speaks commented Nov 25, 2017 •

edited

Loading

bobhaffner commented Nov 26, 2017

jreback left a comment

jreback Nov 26, 2017

jreback Nov 26, 2017

jreback Nov 26, 2017

bobhaffner commented Nov 26, 2017

jreback left a comment

jreback Nov 26, 2017

jreback Nov 26, 2017

bobhaffner Nov 26, 2017

jreback Nov 26, 2017

jreback commented Nov 26, 2017

bobhaffner commented Nov 26, 2017

jreback commented Nov 26, 2017

bobhaffner commented Nov 27, 2017

BUG fixes tuple agg issue 18079 #18354

BUG fixes tuple agg issue 18079 #18354

Conversation

bobhaffner commented Nov 18, 2017 • edited Loading

codecov bot commented Nov 18, 2017 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bobhaffner commented Nov 18, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bobhaffner commented Nov 25, 2017

jreback left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bobhaffner commented Nov 25, 2017

jreback commented Nov 25, 2017

pep8speaks commented Nov 25, 2017 • edited Loading

Comment last updated on November 26, 2017 at 16:18 Hours UTC

bobhaffner commented Nov 26, 2017

jreback left a 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

bobhaffner commented Nov 26, 2017

jreback left a 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 Nov 26, 2017

bobhaffner commented Nov 26, 2017

jreback commented Nov 26, 2017

bobhaffner commented Nov 27, 2017

bobhaffner commented Nov 18, 2017 •

edited

Loading

codecov bot commented Nov 18, 2017 •

edited

Loading

pep8speaks commented Nov 25, 2017 •

edited

Loading