gh-93343: Expand warning filter examples #106618
Conversation
|
All commit authors signed the Contributor License Agreement. |
There was a problem hiding this comment.
Thanks for the PR!
This is a step in the right direction, but there are more subtleties in pytest-dev/pytest#8759 (comment) that are not described here.
In particular:
- Where/when can I uses regexes vs plain text? This applies to both different types of filters (
filterwarnings()/-W/PYTHONWARNINGS) and different fields within a filter (message/package/module). - How to match part of a message with the different types of filters (beginning of the message vs substring).
- How to filter messages coming from specific packages/modules/submodules with the different types of filters.
I think this would be better covered in a new section of the warnings docs -- either a generic Examples section that includes these alongside with comments and explanations, or a more specific section that focuses on the differences between the different warnings types. A mini-howto might also be an option, but we can start with a single section and expand it later.
In addition, since you would have to test different filters to ensure that the behavior you are documenting is correct, it might be worth adding some of these examples to the warnings tests in Lib/test.
| @@ -214,6 +214,18 @@ Some examples:: | |||
| ignore,default:::mymodule # Only report warnings triggered by "mymodule" | |||
| error:::mymodule # Convert warnings to errors in "mymodule" | |||
|
|
|||
| Note that :func:`filterwarnings` filters have subtle differences from :option:`-W` and :envvar:`PYTHONWARNINGS`:: | |||
|
|
|||
| filterwarnings("ignore", message=".*generic", module=r"yourmodule\.submodule") | |||
There was a problem hiding this comment.
| filterwarnings("ignore", message=".*generic", module=r"yourmodule\.submodule") | |
| filterwarnings("ignore", message="\.*generic", module="yourmodule\.submodule") |
Shouldn't this be escaped as well? The r here is not necessary, and its usage should be consistent between the two args (assuming they are both regexes).
There was a problem hiding this comment.
No since the first arg wants to capture strings that contain 'generic' so that the '.' catches everything, while the first arg want to capture 'yourmodule.submodule' specifically, meaning that the '.' actually captures dot and should be escaped
There was a problem hiding this comment.
Well, it sounds to me you should put that explanation into the docs, @daniel-shimon; if it is unclear for a reviewer, it is not going to be clear for the average docs reader ;)
|
|
||
| filterwarnings("ignore", message=".*generic", module=r"yourmodule\.submodule") | ||
| # Ignore warnings in "yourmodule.submodule" which contain "generic" | ||
| filterwarnings("ignore", module="yourmodule.*") |
There was a problem hiding this comment.
| filterwarnings("ignore", module="yourmodule.*") | |
| filterwarnings("ignore", module="yourmodule\.*") |
I think this should be escaped too.
There was a problem hiding this comment.
Also, instead of using comments, would it be clearer (formatting wise) to use multiple code blocks? (Goes for the change below as well)
There was a problem hiding this comment.
No because here we actually want to catch all characters after 'yourmodule' and not only dot
There was a problem hiding this comment.
Misc/NEWS.d/next/Documentation/2023-07-11-08-46-13.gh-issue-93343.fw_eyw.rst
Outdated
Show resolved
Hide resolved
5c2f95e to
826cd01
Compare
Add examples of warning filters and the difference between programatic and environmental filters.
826cd01 to
e43adf8
Compare
|
Hi @ezio-melotti, thanks for the fast review!
I've linked to the warning filter definitions since I think they explain this concisely and don't think we need to add another section repeating the same info. What do you think? |
|
FYI, @daniel-shimon: please don't force push; it does not play well with the GitHub UI, and especially not with review remarks and CI. You can use |
|
Oh, it seems like none of the example code in Doc/library/warnings.rst is checked by doctest :( That is unfortunate! I don't remember the doctest specifics, but if it is possible, please make is so that at least the added examples are run through doctest. If not, adding doctests to this .rst file is out of scope for this PR, but it should be done. |
|
@erlend-aasland so actually this was because I amended the commit, cause I thought it was weird to have "pr fixes" in the main Python repo after accepting the pr. |
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
|
Ok I see and understand that now, thanks guys 🙌 |
np, and thanks for improving the docs! I recommend spending some hours with the devguide; there's a lot of useful information there :) Also, if you find sections in the devguide that are hard to grasp, or just worded badly, don't hesitate to open an issue over at the devguide repo; fixing devguide bugs is one of the most important things we can do :) |
|
Hello from +1 year! Where are we up to on this PR? It looks like there's still some suggestions for @daniel-shimon to address, would you like update the PR? Thanks! |
|
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
|
another half year has passed :) |
|
Hi! |
Add examples of warning filters and the difference between programatic and environmental filters.
warningsdocs #93343📚 Documentation preview 📚: https://cpython-previews--106618.org.readthedocs.build/