-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
TST: Use fixtures in indexes common tests #17622
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
pandas/tests/indexes/test_base.py
Outdated
@@ -29,6 +29,24 @@ | |||
from pandas._libs.lib import Timestamp | |||
|
|||
|
|||
@pytest.fixture(params=[tm.makeUnicodeIndex(100), | |||
tm.makeStringIndex(100), |
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.
move these to a conftest.py in this directory then they can be used in any test files
pandas/tests/indexes/test_base.py
Outdated
@pytest.fixture(params=[tm.makeUnicodeIndex(100), | ||
tm.makeStringIndex(100), | ||
tm.makeDateIndex(100), | ||
tm.makePeriodIndex(100), |
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.
you might also want to call the fixture indices to avoid confusion (then in each test that uses you can either rename index -> indices or do a index = indices
at the top)
pandas/tests/indexes/test_base.py
Outdated
Index([True, False]), | ||
tm.makeCategoricalIndex(100), | ||
Index([]), | ||
MultiIndex.from_tuples(lzip( |
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.
you can name these by:
ids = lambda x: type(x).__name__
can you update |
pandas/conftest.py
Outdated
tm.makeDateIndex(100), | ||
tm.makePeriodIndex(100), | ||
tm.makeTimedeltaIndex(100), | ||
tm.makeIntIndex(100), |
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 should work but i was actually meaning a conftest
in the tests/indexes/conftest.py
these put the fixtures close to the testing code
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.
In case this comes up for anyone else, I got confused about the scope of conftest.py until reading this post: https://stackoverflow.com/questions/34466027/in-py-test-what-is-the-use-of-conftest-py-files
Hello @GuessWhoSamFoo! Thanks for updating the PR. Cheers ! There are no PEP8 issues in this Pull Request. 🍻 Comment last updated on September 24, 2017 at 22:38 Hours UTC |
Codecov Report
@@ Coverage Diff @@
## master #17622 +/- ##
==========================================
+ Coverage 91.2% 91.23% +0.03%
==========================================
Files 163 163
Lines 49637 49806 +169
==========================================
+ Hits 45269 45442 +173
+ Misses 4368 4364 -4
Continue to review full report at Codecov.
|
1 similar comment
Codecov Report
@@ Coverage Diff @@
## master #17622 +/- ##
==========================================
+ Coverage 91.2% 91.23% +0.03%
==========================================
Files 163 163
Lines 49637 49806 +169
==========================================
+ Hits 45269 45442 +173
+ Misses 4368 4364 -4
Continue to review full report at Codecov.
|
thanks! just added about 1000 tests! ideally would like to add more local conftests as needed similar to this. IOW there are other places where we need fixtures similar to this. things like timeseries offsets and others, if you are interested. |
also in pandas/test_base where I think. if you want to search for and make an issue of places to correct would be great (or just send PR's!) |
git diff upstream/master -u -- "*.py" | flake8 --diff
Not sure if there is a better way to define fixture params since they're essentially the indexes defined in
setup_method
. Also thinking about adding an id to each param so the output doesn't look like: