Skip to content

Conversation

adneu
Copy link
Contributor

@adneu adneu commented Apr 25, 2016

In _wrap_applied_output for NDFrameGroupBy, if the first result of apply is None, find the first non-None result to determine behavior

@jreback jreback added Bug Groupby Reshaping Concat, Merge/Join, Stack/Unstack, Explode labels Apr 25, 2016
Copy link
Contributor

Choose a reason for hiding this comment

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

make this a helper function in core/groupby.py (and consolidate the other one which has the same code)

@jreback
Copy link
Contributor

jreback commented May 7, 2016

can you rebase / update

@adneu
Copy link
Contributor Author

adneu commented May 9, 2016

Rebased and made the changes you suggested but there was a new error with the test. It is introduced by #12743 which now throws an error in _concat_objects when any of the values are None (see below). The new PR makes a small change to deal with this, can you check if ok?

======================================================================
ERROR: test_groupby_apply_none_first (pandas.tests.test_groupby.TestGroupBy)
----------------------------------------------------------------------
Traceback (most recent call last):
  File ".../pandas/tests/test_groupby.py", line 6292, in test_groupby_apply_none_first
    result = test_df.groupby('groups').apply(test_func)
  File ".../pandas/core/groupby.py", line 651, in apply
    return self._python_apply_general(f)
  File ".../pandas/core/groupby.py", line 660, in _python_apply_general
    not_indexed_same=mutated or self.mutated)
  File ".../pandas/core/groupby.py", line 3241, in _wrap_applied_output
    not_indexed_same=not_indexed_same)
  File ".../pandas/core/groupby.py", line 824, in _concat_objects
    values = reset_identity(values)
  File ".../pandas/core/groupby.py", line 809, in reset_identity
    ax = v._get_axis(self.axis)
AttributeError: 'NoneType' object has no attribute '_get_axis'

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

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can remove the _count_not_none(....)

@codecov-io
Copy link

codecov-io commented May 9, 2016

Current coverage is 84.12%

Merging #12977 into master will increase coverage by <.01%

@@             master     #12977   diff @@
==========================================
  Files           138        138          
  Lines         50388      50390     +2   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits          42386      42388     +2   
  Misses         8002       8002          
  Partials          0          0          
  1. 4 files (not in diff) in pandas were modified. more
    • Hits -4

Powered by Codecov. Last updated by 4e4a7d9...c630c8b

@jreback
Copy link
Contributor

jreback commented May 16, 2016

can you squash

@jreback jreback added this to the 0.18.2 milestone May 16, 2016
…her first result is None

BUG: GH12824 fixed apply() returns different result depending on whether first result is None

BUG: GH12824 rebased and made requested fixes

BUG: GH12824 made requested change and added tests
@jreback jreback closed this in cc25040 May 20, 2016
@jreback
Copy link
Contributor

jreback commented May 20, 2016

thanks @adneu

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Bug Groupby Reshaping Concat, Merge/Join, Stack/Unstack, Explode

Projects

None yet

Development

Successfully merging this pull request may close these issues.

groupby().apply() returns different result depends on the first result is None or not.

3 participants