-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
STY: use pytest.raises context syntax #24297
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
Hello @jbrockmendel! Thanks for submitting the PR.
|
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.
can you create a lint rule to check for non-context manager usage of pytest.raises
?
pandas/tests/frame/test_indexing.py
Outdated
# - assign a complete row (mixed values) -> exp_single_row | ||
df = orig.copy() | ||
df.iloc[2, :] = ["b", 2] | ||
tm.assert_frame_equal(df, exp_single_row) | ||
|
||
# - assign a complete row (mixed values) not in categories set | ||
def f(): | ||
with pytest.raises(ValueError): | ||
np.log(s) |
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.
what's this
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.
must be a copy/paste mixup, will fix
Codecov Report
@@ Coverage Diff @@
## master #24297 +/- ##
===========================================
- Coverage 92.28% 43% -49.28%
===========================================
Files 162 162
Lines 51830 51830
===========================================
- Hits 47831 22290 -25541
- Misses 3999 29540 +25541
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #24297 +/- ##
=======================================
Coverage 92.28% 92.28%
=======================================
Files 162 162
Lines 51827 51827
=======================================
Hits 47827 47827
Misses 4000 4000
Continue to review full report at Codecov.
|
@jbrockmendel : Thanks for the ping. Given how massive this is, I think we can just focus on the conversion to context manager first. Which ones should use a match parameter can be determined after (i.e. subsequent PR’s) |
thanks. @gfyoung we should prob have a lint rule to detect this, but leave commented for now |
@jreback: I actually was discussing the checking if error messages, not the usage of the context manager versus not. That being said, I do agree we should lint for what was addressed here. |
There are still about a thousand non-context usages of pytest.raises.
@gfyoung if there is a subset of these that especially merit match kwargs, let me know. I'm not eager to manually go through all of them, but doing something systematic or manually doing a few I'd be up for.