REF/TST: Add more pytest idiom to indexing/multiindex/test_getitem.py #24452
Conversation
Codecov Report
@@ Coverage Diff @@
## master #24452 +/- ##
==========================================
+ Coverage 92.3% 92.31% +<.01%
==========================================
Files 163 163
Lines 51987 51987
==========================================
+ Hits 47989 47990 +1
+ Misses 3998 3997 -1
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #24452 +/- ##
==========================================
- Coverage 92.3% 92.3% -0.01%
==========================================
Files 163 163
Lines 51987 51969 -18
==========================================
- Hits 47989 47968 -21
- Misses 3998 4001 +3
Continue to review full report at Codecov.
|
|
|
||
|
|
||
| def test_getitem_int(multiindex_dataframe_random_data): | ||
| @pytest.fixture |
There was a problem hiding this comment.
ideally put the fixtures at the top
| frame = multiindex_dataframe_random_data.T | ||
| expected = frame | ||
| df = frame.copy() | ||
|
|
||
| try: |
There was a problem hiding this comment.
don't do things like this; use the context manager like above
There was a problem hiding this comment.
i've changed as requested. though not convinced this is the right thing to do in situations like this. the previous test tested that the chained assignment raised the correct exception and this test is to check that the dataframe values are unchanged. having a context manager in the test adds unnecessary/unseen setup and teardown code that could potentially interfere with the tested functionality.
There was a problem hiding this comment.
yeah this must have been changed a while back, the context manager is the correct idiom
|
thanks @simonjayhawkins |
git diff upstream/master -u -- "*.py" | flake8 --diffin this pass: