fix: warn_deprecated now respect warnings.filter ignores#1476
Open
hularuns wants to merge 9 commits into
Open
Conversation
…arnings - checks by checking module name and action
hularuns
changed the title
ignore deprecation w…
Author
|
I'm not sure how the pyright check is failing |
Enegg
reviewed
Nov 14, 2025
Contributor
|
@onerandomusername pyright tests are borked |
Enegg
reviewed
Nov 14, 2025
Contributor
Enegg
left a comment
Enegg
left a comment
There was a problem hiding this comment.
I'm not sure we really need this hack to be in the lib in the first place, but since this is legacy behavior I'm not one to decide that
Comment on lines
+316
to
+324
| if send_warning: | ||
| # this still allows force bypassing of filters if the default DeprecationWarning ignore is set. | ||
| try: | ||
| warnings.simplefilter(action="always", category=DeprecationWarning) | ||
| warnings.warn(*args, stacklevel=stacklevel + 1, category=DeprecationWarning, **kwargs) | ||
| finally: | ||
| # NOTE: Is this assertion even necessary? warnings.filters is always at minimum an empty list? | ||
| assert isinstance(warnings.filters, list) | ||
| warnings.filters[:] = old_filters |
Contributor
There was a problem hiding this comment.
Suggested change
| if send_warning: | |
| # this still allows force bypassing of filters if the default DeprecationWarning ignore is set. | |
| try: | |
| warnings.simplefilter(action="always", category=DeprecationWarning) | |
| warnings.warn(*args, stacklevel=stacklevel + 1, category=DeprecationWarning, **kwargs) | |
| finally: | |
| # NOTE: Is this assertion even necessary? warnings.filters is always at minimum an empty list? | |
| assert isinstance(warnings.filters, list) | |
| warnings.filters[:] = old_filters | |
| # allow force bypassing of filters if the default DeprecationWarning ignore is set. | |
| if not send_warning: | |
| return | |
| try: | |
| warnings.simplefilter(action="always", category=DeprecationWarning) | |
| warnings.warn(*args, stacklevel=stacklevel + 1, category=DeprecationWarning, **kwargs) | |
| finally: | |
| assert isinstance(warnings.filters, list) | |
| warnings.filters[:] = old_filters |
Author
There was a problem hiding this comment.
agree on the guard clause instead and committed your suggestion. Just thought, after committing we could instead return within the for loop of old_filters instead of assigning send_warning a value and breaking from the loop. Personally, I don't like that as it's not as explicit
Contributor
There was a problem hiding this comment.
The bool variable can be avoided by using a for ... else clause, while still keeping it flat.
for ... in ...:
if condition:
break
else:
return
# here goes stuff that should run when condition is true
Author
There was a problem hiding this comment.
Instead of the early break, i've settled for doing an early return here instead
Co-authored-by: Eneg <42005170+Enegg@users.noreply.github.com> Signed-off-by: Sam <159131400+hularuns@users.noreply.github.com>
Co-authored-by: Eneg <42005170+Enegg@users.noreply.github.com> Signed-off-by: Sam <159131400+hularuns@users.noreply.github.com>
- updated tests to be more explanatory if they fail assertions
- also removed the cheeky change of old_filters >= and replaced with >
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
4 participants
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.
Summary
Closes #1475
warn_deprecated now respect if a user wants to ignore deprecation warnings by checking if the module name has
disnakein it.test_deprecated_warntests have been added.Expected use will be:
Using simplefilter or not passing a module value will result in the existing behaviour now which forces through the warning, as such this change will not impact current users unless they use the above example, or similar.
warning filters are generally not that large, so there is expected to be a very negligible impact to performance.
Known potential issues with this approach
ignoreaction value for the warnings.disnakein it will be picked up to be ignored if the action isignoreChecklist
uv run nox -s lintuv run nox -s pyright