STY: Check pytest.raises is used context manager #24332
Conversation
gfyoung
added
CI
Codecov Report
@@ Coverage Diff @@
## master #24332 +/- ##
==========================================
+ Coverage 92.28% 92.28% +<.01%
==========================================
Files 162 162
Lines 51831 51808 -23
==========================================
- Hits 47830 47813 -17
+ Misses 4001 3995 -6
Continue to review full report at Codecov.
|
|
The check works as expected: https://dev.azure.com/pandas-dev/pandas/_build/results?buildId=5501 I will address comments from @datapythonista and comment out the check. |
|
There’s still something like a thousand non-compliant usages right? |
@gfyoung right, don't we need to either comment this out or run it but not fail on it? |
|
@jreback : As I mentioned above, I will comment out the check now that I have confirmed that it works. @jbrockmendel : Indeed, there are about 1000 non-compliant usages (1034 to be exact if you run the |
|
@gfyoung sgtm. |
|
@jreback @datapythonista : Addressed all comments, and everything is green. PTAL. |
There was a problem hiding this comment.
lgtm
not sure what's your plan next, but my suggestion is to fix few cases yourself in a file or directory, activate those in this validation, and then create some issues labelled as "good first issue", so first time contributors have the opportunity to work with something as easy as this (and use your first PR as a reference, which will make things even easier for them).
Or if you really prefer to fix all them yourself, please do it in several PRs (I read that we've got some thousands of cases, I'm assuming that was said literally)
|
thanks @gfyoung yeah prob need a way to actually implement this. I think a few PR's might just do it |
|
This effort will likely intersect with some other work I have been doing to add more |
|
For reference, running the command: yields: pandas/tests/arithmetic/test_timedelta64.py |
Specifically, use context manager for pytest.raises. xref pandas-devgh-24332.
Specifically, use context manager for pytest.raises. xref gh-24332.
Specifically, use context manager for pytest.raises. xref pandas-devgh-24332.
Specifically, use context manager for pytest.raises. xref pandas-devgh-24332.
|
@gfyoung : will probably need to ignore
would work. also may need to ignore |
|
@simonjayhawkins : You're right that those are edge cases we would have to tackle. However, as we're in no rush to invoke this linting yet, let's see how many of those we have (especially once |
OK. that's simpler, i'll go with that approach if no objections.
2 PRS awaiting approval, 1 PR should cover frame and another pytables, then moreless done. |
|
following #25516 now yields
|
* STY: Check for pytest.raises without context xref gh-24332 * CLN: Remove remaining non-context pytest.raises
Follow-up to #24297 (comment).