Skip to content

gh-129846: clarify suppression of warning output in catch_warnings#130768

Open
Mr-Sunglasses wants to merge 11 commits into
python:mainfrom
Mr-Sunglasses:fix/129846
Open

gh-129846: clarify suppression of warning output in catch_warnings#130768
Mr-Sunglasses wants to merge 11 commits into
python:mainfrom
Mr-Sunglasses:fix/129846

Conversation

@Mr-Sunglasses
Copy link
Copy Markdown
Contributor

@Mr-Sunglasses Mr-Sunglasses commented Mar 2, 2025

@bedevere-app bedevere-app Bot added docs Documentation in the Doc dir skip news awaiting review labels Mar 2, 2025
@github-project-automation github-project-automation Bot moved this to Todo in Docs PRs Mar 2, 2025
@Mr-Sunglasses Mr-Sunglasses changed the title gh-129846: docs(warnings): clarify suppression of warning output in `… h-129846: docs(warnings): clarify suppression of warning output in catch_warnings Mar 2, 2025
@Mr-Sunglasses Mr-Sunglasses changed the title h-129846: docs(warnings): clarify suppression of warning output in catch_warnings gh-129846: docs(warnings): clarify suppression of warning output in catch_warnings Mar 2, 2025
Copy link
Copy Markdown
Member

@StanFromIreland StanFromIreland left a comment

Choose a reason for hiding this comment

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

I think we should drop the note and just integrate it above.

Comment thread Doc/library/warnings.rst Outdated
Mr-Sunglasses and others added 2 commits March 4, 2025 00:00
Co-authored-by: Stan Ulbrych <89152624+StanFromIreland@users.noreply.github.com>
@Mr-Sunglasses
Copy link
Copy Markdown
Contributor Author

tagging @sobolevn for a review. TiA.

@github-actions
Copy link
Copy Markdown

This PR is stale because it has been open for 30 days with no activity.

@github-actions github-actions Bot added the stale Stale PR or inactive for long period of time. label Apr 22, 2026
Copy link
Copy Markdown
Member

@serhiy-storchaka serhiy-storchaka left a comment

Choose a reason for hiding this comment

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

While we are here, I suggest few more changes.

  • Replace :const:`False` and :const:`True` with simple false and true. Any value is accepted, not only these singletons.
  • Replace :class:`None` with ``None``. It is not a class.
  • Start the new sentence from the new line. We change this line in any case, this will minimize changed lines in future.
  • Mention that :func:`showwarning` is reset to the default implementation.
  • Maybe the sentence can be changed more, it looks complicated like "The House That Jack Built".

@github-actions github-actions Bot removed the stale Stale PR or inactive for long period of time. label May 23, 2026
@serhiy-storchaka
Copy link
Copy Markdown
Member

@StanFromIreland, could you please review the current variant?

@read-the-docs-community
Copy link
Copy Markdown

read-the-docs-community Bot commented May 30, 2026

Documentation build overview

📚 cpython-previews | 🛠️ Build #32918634 | 📁 Comparing 875e10d against main (2c20f9c)

  🔍 Preview build  

3 files changed
± library/binascii.html
± library/warnings.html
± whatsnew/changelog.html

@picnixz picnixz changed the title gh-129846: docs(warnings): clarify suppression of warning output in catch_warnings gh-129846: d clarify suppression of warning output in catch_warnings May 30, 2026
@picnixz picnixz changed the title gh-129846: d clarify suppression of warning output in catch_warnings gh-129846: clarify suppression of warning output in catch_warnings May 30, 2026
@picnixz
Copy link
Copy Markdown
Member

picnixz commented May 30, 2026

Note: we don't use conventional commits headings

Comment thread Doc/library/warnings.rst Outdated
Comment thread Doc/library/warnings.rst Outdated
Comment thread Doc/library/warnings.rst
false, the context manager works by replacing and then later restoring the
module's :func:`showwarning` function. That is not concurrent-safe.

When *record* is true and the flag is true, the :func:`showwarning` function
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hmm, should this be fixed too:

cpython/Lib/_py_warnings.py

Lines 712 to 720 in 56bd9ea

if self._record:
if _use_context:
context.log = log = []
else:
log = []
self._module._showwarnmsg_impl = log.append
# Reset showwarning() to the default implementation to make sure
# that _showwarnmsg() calls _showwarnmsg_impl()
self._module.showwarning = self._module._showwarning_orig

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The code does not match the documentation. This can be a separate bug.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This was changed in #146358, but now not only the code does not match docs, but I am not sure that context_aware_warnings have not lost its meaning.

Co-authored-by: Stan Ulbrych <stan@python.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting review docs Documentation in the Doc dir skip news

Projects

Status: Todo

Development

Successfully merging this pull request may close these issues.

4 participants