Skip to content

gh-103109: Add documentation for ignore_warnings #103110

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

Merged
merged 5 commits into from
Apr 2, 2023

Conversation

CharlieZhao95
Copy link
Contributor

@CharlieZhao95 CharlieZhao95 commented Mar 29, 2023

  • Add documentation for ignore_warnings
  • Improve the comment in function ignore_warnings

Copy link
Member

@CAM-Gerlach CAM-Gerlach left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Standard reminder: You can directly apply the suggestion below by simply clicking Commit suggestion, entering a descriptive commit message and then clicking Commit changes

One fix for some textual errors/issues, otherwise LGTM - thanks @CharlieZhao95 !

Co-authored-by: C.A.M. Gerlach <[email protected]>
Copy link
Member

@CAM-Gerlach CAM-Gerlach left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks @CharlieZhao95 !

Copy link
Contributor

@erlend-aasland erlend-aasland left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a versionadded:: 3.8. Also, I think there's some relevant information in the docstring that should be included in the docs, namely that this decorator should be used with care since tools like git blame may become less useful with warnings suppressed. Forget that last part; too little coffee for me! ☕ ☕ ☕

Also note that there's a targeted variant called ignore_warnings_from (that's also undocumented1).

Footnotes

  1. Note that a lot of the test support functions are undocumented, some deliberately so, simply because they're mostly for internal use.

@bedevere-bot
Copy link

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.

@erlend-aasland
Copy link
Contributor

Oh BTW, would you mind reflowing this according to SemBr (within 79 chars line width)? If not, I'll do that before merging.

@CharlieZhao95
Copy link
Contributor Author

would you mind reflowing this according to SemBr (within 79 chars line width)? If not, I'll do that before merging.

Oh! Thanks for your suggestion. I love these nits! This doesn't look too hard, I can try to get it done, and I'll try to follow the SemBr spec going forward. Improvements are always good! 😃

@CharlieZhao95
Copy link
Contributor Author

Also note that there's a targeted variant called ignore_warnings_from (that's also undocumented1).

BTW, I didn't find the code that matches ignore_warnings_from, please let me know if it is my problem. :)

@CAM-Gerlach
Copy link
Member

BTW, I didn't find the code that matches ignore_warnings_from, please let me know if it is my problem. :)

FWIW I couldn't find it either, grepping the codebase for that string and also searching the repo, issues, commits, PRs, etc. on GitHub (except for this one, of course).

@erlend-aasland
Copy link
Contributor

erlend-aasland commented Mar 31, 2023

Sorry, that was a typo on my side :) It's ignore_deprecations_from:

def ignore_deprecations_from(module: str, *, like: str) -> object:
token = object()
warnings.filterwarnings(
"ignore",
category=DeprecationWarning,
module=module,
message=like + fr"(?#support{id(token)})",
)
return token

And, as far as I can see, it is only used in the tests for test.support, so there is no need to document it.

Co-authored-by: C.A.M. Gerlach <[email protected]>
Copy link
Member

@CAM-Gerlach CAM-Gerlach left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM again, thanks @CharlieZhao95

@miss-islington
Copy link
Contributor

Thanks @CharlieZhao95 for the PR, and @erlend-aasland for merging it 🌮🎉.. I'm working now to backport this PR to: 3.10, 3.11.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Apr 2, 2023
…onGH-103110)

(cherry picked from commit 32937d6)

Co-authored-by: Charlie Zhao <[email protected]>
Co-authored-by: C.A.M. Gerlach <[email protected]>
@bedevere-bot
Copy link

GH-103198 is a backport of this pull request to the 3.11 branch.

@bedevere-bot bedevere-bot removed the needs backport to 3.11 only security fixes label Apr 2, 2023
@bedevere-bot
Copy link

GH-103199 is a backport of this pull request to the 3.10 branch.

@bedevere-bot bedevere-bot removed the needs backport to 3.10 only security fixes label Apr 2, 2023
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Apr 2, 2023
…onGH-103110)

(cherry picked from commit 32937d6)

Co-authored-by: Charlie Zhao <[email protected]>
Co-authored-by: C.A.M. Gerlach <[email protected]>
miss-islington added a commit that referenced this pull request Apr 2, 2023
(cherry picked from commit 32937d6)

Co-authored-by: Charlie Zhao <[email protected]>
Co-authored-by: C.A.M. Gerlach <[email protected]>
miss-islington added a commit that referenced this pull request Apr 2, 2023
(cherry picked from commit 32937d6)

Co-authored-by: Charlie Zhao <[email protected]>
Co-authored-by: C.A.M. Gerlach <[email protected]>
@Security2965 Security2965 linked an issue Apr 7, 2023 that may be closed by this pull request
@arhadthedev arhadthedev removed a link to an issue Apr 7, 2023
gaogaotiantian pushed a commit to gaogaotiantian/cpython that referenced this pull request Apr 8, 2023
warsaw pushed a commit to warsaw/cpython that referenced this pull request Apr 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Documentation in the Doc dir skip news
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants