-
-
Notifications
You must be signed in to change notification settings - Fork 18.5k
Break apart test_frame.py and fix all flake8 warnings #12032
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
Conversation
…nd fix flake8 errors on that code
lgtm at first glance. |
What's the story with the pickle-related test failures? |
aren't any in master - do u have a run where something fails? |
OK, I'm done, finally. before
after
30 second gain is pretty good; mostly by trimming unnecessary I seem to have somehow dropped 38 unit tests, so let me figure out if I lost any real test code (there were a bunch of empty test cases) by accident |
These are the only test names that aren't being run in the refactored version:
So I think we are good to go if/when Jenkins passes |
I guess this patch broke GitHub. here's the diff stats i'm seeing
|
|
@jreback here's an example of what I was talking about https://travis-ci.org/wesm/pandas/jobs/102231014 any idea what is going on there? |
It has something to do with the Travis configuration for personal branch test runs |
@wesm that is very weird. That occurs only on your personal travis-ci ? (but ok on pydata/pandas)? |
Seems that way. If this build passes, recommend merging soon to avoid too many merge conflicts |
sure, merge on pass |
Bombs away. What tool are you using for squash-merging nowadays? A lot of Apache projects have moved to cherry-pick only with no merge commits. |
Break apart test_frame.py and fix all flake8 warnings
I usually just squash to a single commit and merge. |
@jreback I re-enabled flake8 checking for unused imports. We should be explicit about what imports are part of a module API (and those warnings explicitly suppressed with
# noqa
) and delete all unused imports.