-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
CLN: Move info #31876
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
CLN: Move info #31876
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm @jreback
44efd34
to
6641e2b
Compare
@MarcoGorelli can you merge master to resolve conflicts |
@simonjayhawkins sure, done |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pls don't mix fixturing with a move. this is too confusing. just do these move in this PR; you can make another one for the other changes.
faf2252
to
b956d2a
Compare
Hello @MarcoGorelli! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found: There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻 Comment last updated at 2020-02-18 10:21:23 UTC |
@jreback sure, have reverted renaming the fixture and have added type annotations |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks ok, ping on green.
233f78c
to
5597914
Compare
@jreback sure, have moved all imports to the top and have reverted moving the fixture |
thanks @MarcoGorelli move PRs have to be exactly a move only (and whatever fixes are needed in the target files to get green CI), otherwise very difficult / impossible to reveiw. |
precursor to xref #31796. Have moved
DataFrame.info
code intopandas/io/formats/info.py
, along with tests