TST: enforce that assert_produces_warning checks the warning message - #67275
Merged
Conversation
Closes GH#58290. Adds a `bare_assert_produces_warning` check to scripts/validate_unwanted_patterns.py, wired up as a pre-commit hook over pandas/tests. It flags `tm.assert_produces_warning(SomeWarning)` (and the `maybe_produces_warning` wrapper, which forwards **kwargs into it) when no `match` argument is given. Calls that assert no warning is raised -- `assert_produces_warning(None)` and `(False)` -- are exempt, and an explicit `match=None` opts out where there is genuinely no message to assert. That is what the helper's own tests use: several either expect a falsy warning class or never emit the expected class at all, so the message assertion is unreachable by construction. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
jbrockmendel
force-pushed
the
tst-58290-lint
branch
from
August 29, 2026 22:03
fa7e9dc to
a0f7100
Compare
jbrockmendel
marked this pull request as ready for review
August 29, 2026 22:03
mroeschke
reviewed
Aug 30, 2026
Comment on lines
+266
to
+271
| - id: unwanted-patterns-bare-assert-produces-warning | ||
| name: Check that assert_produces_warning also checks the warning message | ||
| language: python | ||
| entry: python scripts/validate_unwanted_patterns.py --validation-type="bare_assert_produces_warning" | ||
| types: [python] | ||
| files: ^pandas/tests/ |
Member
There was a problem hiding this comment.
Alternatively, could we change the signature of assert_produces_warning make expected_warning and match required arguments?
Member
Author
There was a problem hiding this comment.
Good idea, will make a follow-up.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The enforcement half of GH-58290. The four content PRs it depended on — GH-67270, GH-67271, GH-67273, GH-67274 — have all merged, so this now runs clean; rebased onto their tips.
Adds a
bare_assert_produces_warningcheck toscripts/validate_unwanted_patterns.py, wired up as a pre-commit hook scoped topandas/tests/. It flagstm.assert_produces_warning(SomeWarning)with nomatchargument.It also covers
tm.maybe_produces_warning, which forwards**kwargsstraight intoassert_produces_warning. Without that, swapping one helper for the other is a one-word way around the check.Exemptions. Calls asserting that nothing is raised —
assert_produces_warning(None)and(False)— are exempt, since there is no message to match. Beyond that an explicitmatch=Noneopts out, which is roughly thetm.external_warning_producedescape hatch suggested in the issue;# pdlint: ignore[bare_warning_match]also works, following the existingpdlintconvention in this script.The only place needing an exemption today is the helper's own test file, and in each case the message assertion is unreachable by construction rather than merely inconvenient:
TestFalseOrNoneExpectedWarningparametrizesfalse_or_noneover[False, None], so no warning is expected at all — but the value is a name, not a literal, so a static check cannot see that.test_fail_to_catch_actual_warningandtest_raises_during_exceptionnever emit the expected class, soassert_produces_warningraisesDid not see expected warning of class ...before it ever looks atmatch— which is exactly what those tests assert.The other four calls in that file did have an assertable message and now check it.
Verified against the full tree.
pre-commit run --all-filespasses on the rebased branch with no files modified, so the check reports zero violations across every file inpandas/tests/and lands with no exclusion list. (Cross-checked that it is not silently passing by feeding it a deliberately bare call, which it flags.)scripts/tests/test_validate_unwanted_patterns.pycovers both helper names, thetm.-qualified and bare forms, each exempt spelling, the implicitWarningdefault, and thepdlintcomment.