-
-
Notifications
You must be signed in to change notification settings - Fork 32.8k
bpo-16845: Validate the category of warnings.simplefilter sooner #31983
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
Co-authored-by: Berker Peksag <berker.peksag@gmail.com>
@@ -274,7 +274,11 @@ def __enter__(self): | |||
v.__warningregistry__ = {} | |||
self.warnings_manager = warnings.catch_warnings(record=True) | |||
self.warnings = self.warnings_manager.__enter__() | |||
warnings.simplefilter("always", self.expected) | |||
if isinstance(self.expected, tuple): |
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.
This should probably be done in a separate PR, but issubclass
does not support a tuple. However, the type of the category
parameter of warnings.simplefilter
doesn't really indicate that it should support a tuple anyway.
I could also change it so that the issubclass
checks for tuples
before doing the check, but I'm not sure if that is actually correct.
Let me know!
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.
I think that there may be a third-party code which uses simplefilter()
with a tuple. Instead of changing unittest, it is better to make the argument check in simplefilter()
more lenient.
It would be nice to make also filterwarnings()
accepting a tuple.
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.
Ran 117 tests in 2.152s
OK
Looks ok to me.
@berkerpeksag Gentle ping as this is basically your patch from https://bugs.python.org/issue16845 > https://bugs.python.org/file35773/issue16845.diff. Sorry if such pings are not appreciated! |
I'm closing this PR. On master @serhiy-storchaka if you agree do you mind closing the associated issue? |
This is a submit of the patch provided by @ berkerpeksag in bpo-16845. I added one additional test case.
Not sure if I should ping them here.
It supersedes #26696 which did not received any interaction after the initial push and has now been marked stale.
https://bugs.python.org/issue16845
Closes #61049