Skip to content

Conversation

@jbrockmendel
Copy link
Member

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

@WillAyd
Copy link
Member

WillAyd commented Aug 28, 2019

If you are deep diving on this any chance you can add a docstring for func to help future readers?

Copy link
Member

@WillAyd WillAyd left a comment

Choose a reason for hiding this comment

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

minor nit but otherwise lgtm - I think this certainly reads nicer

if not isinstance(res_fast, DataFrame):
return path, res

if not res_fast.columns.equals(group.columns):
Copy link
Member

Choose a reason for hiding this comment

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

Can you add this condition to the check above?

Copy link
Member Author

Choose a reason for hiding this comment

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

for coverage purposes Id rather leave them separate. i.e. i want to be able to see that both cases are reached

@WillAyd WillAyd added this to the 1.0 milestone Aug 30, 2019
@jreback
Copy link
Contributor

jreback commented Sep 2, 2019

lgtm. @WillAyd ?

@WillAyd WillAyd merged commit 562f423 into pandas-dev:master Sep 2, 2019
@WillAyd
Copy link
Member

WillAyd commented Sep 2, 2019

Thanks @jbrockmendel

@jbrockmendel jbrockmendel deleted the choose_path branch September 2, 2019 22:54
MarcoGorelli pushed a commit to MarcoGorelli/pandas that referenced this pull request Sep 3, 2019
proost pushed a commit to proost/pandas that referenced this pull request Dec 19, 2019
proost pushed a commit to proost/pandas that referenced this pull request Dec 19, 2019
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