STY: use pytest.raises context syntax#24297
Conversation
|
Hello @jbrockmendel! Thanks for submitting the PR.
|
jreback
left a comment
There was a problem hiding this comment.
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) not in categories set | ||
| def f(): | ||
| with pytest.raises(ValueError): | ||
| np.log(s) |
There was a problem hiding this comment.
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.