Skip to content

Removed Need for OHLC As First Element if Used in .agg #21769

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 5 commits into from
Jul 7, 2018

Conversation

WillAyd
Copy link
Member

@WillAyd WillAyd commented Jul 6, 2018

# let higher level handle
if _level:
return results
return list(compat.itervalues(results))[0]
Copy link
Member Author

Choose a reason for hiding this comment

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

There was no test coverage for this line and I can't feasibly think how this would be useful, hence the deletion

@jreback jreback added this to the 0.23.3 milestone Jul 6, 2018
Copy link
Contributor

@jreback jreback left a comment

Choose a reason for hiding this comment

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

lgtm. pls add a whatsnew note in 0.23.3.



def test_groupby_agg_ohlc_non_first():
df = pd.DataFrame([[1], [1]], columns=['foo'],
Copy link
Contributor

Choose a reason for hiding this comment

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

can you add the issue number as a comment

@jreback
Copy link
Contributor

jreback commented Jul 6, 2018

lgtm. merge on green.

@WillAyd
Copy link
Member Author

WillAyd commented Jul 7, 2018

CircleCI failure was a resource warning unrelated to PR

@WillAyd WillAyd merged commit de664e8 into pandas-dev:master Jul 7, 2018
@WillAyd WillAyd deleted the ohlc-agg branch July 7, 2018 13:44
lumberbot-app bot pushed a commit that referenced this pull request Jul 7, 2018
Sup3rGeo pushed a commit to Sup3rGeo/pandas that referenced this pull request Oct 1, 2018
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.

GroupBy OHLC Error When Used in .agg with Other Functions
2 participants