Skip to content

CLN: _wrap_applied_output #36260

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Sep 12, 2020
Merged

Conversation

rhshadrach
Copy link
Member

  • closes #xxxx
  • tests added / passed
  • passes black pandas
  • passes git diff upstream/master -u -- "*.py" | flake8 --diff
  • whatsnew entry

else:
result = DataFrame(values, index=key_index, columns=[self._selection])
self._insert_inaxis_grouper_inplace(result)
return result
Copy link
Member

Choose a reason for hiding this comment

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

looks like this line isnt covered (the equivalent line isnt covered in master, so not necessarily a problem for this PR). any chance it is unreachable and can be removed?

Copy link
Member Author

Choose a reason for hiding this comment

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

Added test to cover it.

@jbrockmendel
Copy link
Member

Changing v to first_not_none is a clear improvement. the other rearrangement im fine either way

@rhshadrach
Copy link
Member Author

@jbrockmendel: Thanks for the review. Regarding the rearrangement, I thought it was better to separate "not a series" and "is a series" cases. Before they were (needlessly IMO) intertwined.

@jreback jreback added this to the 1.2 milestone Sep 12, 2020
@jreback jreback merged commit 39c5e29 into pandas-dev:master Sep 12, 2020
@jreback
Copy link
Contributor

jreback commented Sep 12, 2020

thanks @rhshadrach yeah groupby has built up over the years to be long / complicated functions, thanks for working on this.

@rhshadrach rhshadrach deleted the _wrap_applied_output branch September 12, 2020 21:15
kesmit13 pushed a commit to kesmit13/pandas that referenced this pull request Nov 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants