-
-
Notifications
You must be signed in to change notification settings - Fork 18.5k
CLN: Fixturize test_base.py #23943
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: Fixturize test_base.py #23943
Conversation
Hello @nixphix! Thanks for updating the PR. Cheers ! There are no PEP8 issues in this Pull Request. 🍻 Comment last updated on January 01, 2019 at 15:55 Hours UTC |
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.
prob you just want to do the move & rename of the fixtures in a PR w/o actually changing anything else. Then in a future PR you can modify things, otherwise the diff is going to be pretty big.
@jreback could please review the changes |
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 merge master as well
can you rebase and update |
Codecov Report
@@ Coverage Diff @@
## master #23943 +/- ##
===========================================
- Coverage 92.31% 43.07% -49.25%
===========================================
Files 166 166
Lines 52335 52339 +4
===========================================
- Hits 48313 22543 -25770
- Misses 4022 29796 +25774
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #23943 +/- ##
===========================================
+ Coverage 31.88% 92.3% +60.41%
===========================================
Files 166 166
Lines 52413 52416 +3
===========================================
+ Hits 16714 48384 +31670
+ Misses 35699 4032 -31667
Continue to review full report at Codecov.
|
@jreback all comments are resolved and checks passed |
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.
this looks pretty good. can you also add an ids
for the fixtures so we have nice names as well (you might have to get creative to do this, e.g. parsing the function names0
@jreback could you help me figure out what's causing build failure in py 3.7 The same test case passes in master while it's failing in the pull request. Master pandas/pandas/tests/test_base.py Lines 270 to 278 in c1be6b1
Pull Request pandas/pandas/tests/test_base.py Lines 266 to 271 in bd4e246
Much appreciate your help |
@nixphix can you merge master and check failures? |
Closing as stale. Ping if you'd like to continue |
Partially Fixes #23877
git diff upstream/master -u -- "*.py" | flake8 --diff
@jreback Could you please review the fixtures, I haven't replaced all the
indexes
if the changes are correct I would fixturize other test cases