| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
Sorry, something went wrong.
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:
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.
Sorry, something went wrong.
|
|
||
| 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).
Sorry, something went wrong.
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
Sorry, something went wrong.
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 ;)
Sorry, something went wrong.
Add examples of warning filters and the difference between programatic and environmental filters.
|
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? |
Sorry, something went wrong.
|
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 git pull --no-ff main instead. See also https://devguide.python.org/getting-started/pull-request-lifecycle/ |
Sorry, something went wrong.
|
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. |
Sorry, something went wrong.
|
@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. |
Sorry, something went wrong.
|
Ok I see and understand that now, thanks guys 🙌 |
Sorry, something went wrong.
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 :) |
Sorry, something went wrong.
|
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! |
Sorry, something went wrong.
There was a problem hiding this comment.
Ping :)
Sorry, something went wrong.
|
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 I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request. |
Sorry, something went wrong.
|
another half year has passed :) |
Sorry, something went wrong.
|
Hi! |
Sorry, something went wrong.
|
Hi @erlend-aasland , I added clarifications regarding escaping and regexes. |
Sorry, something went wrong.
| # in order to match a literal dot character. | ||
| filterwarnings("ignore", message="generic", module=r"yourmodule\.submodule") |
There was a problem hiding this comment.
I think those examples should be separated by a new line for clarity purposes.
Sorry, something went wrong.
|
|
||
| def test_string_literals(self): | ||
| # Ensure message/module are treated as string literals | ||
| rc, stdout, stderr = assert_python_ok("-c", |
There was a problem hiding this comment.
If you use rc, check its value as well. Otherwise use _, stdout, stderr = ...
Sorry, something went wrong.
There was a problem hiding this comment.
Fix CR Comments
Sorry, something went wrong.
|
Hi @picnixz, thanks for the review! I've fixed your comments 🙏 |
Sorry, something went wrong.
|
This PR is stale because it has been open for 30 days with no activity. |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
Add examples of warning filters and the difference between programatic and environmental filters.
📚 Documentation preview 📚: https://cpython-previews--106618.org.readthedocs.build/