Skip to content

bpo-39796: Fix _warnings module initialization #18739

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Mar 2, 2020
Merged

bpo-39796: Fix _warnings module initialization #18739

merged 1 commit into from
Mar 2, 2020

Conversation

vstinner
Copy link
Member

@vstinner vstinner commented Mar 2, 2020 •
edited by bedevere-bot
Loading

  • Add _PyWarnings_InitState() which only initializes the _warnings
    module state (tstate->interp->warnings) without creating a module
    object
  • Py_InitializeFromConfig() now calls _PyWarnings_InitState() instead
    of _PyWarnings_Init()
  • Rename also private functions of _warnings.c to avoid confusion
    between the public C API and the private C API.

https://bugs.python.org/issue39796

@vstinner
bpo-39796: Fix _warnings module initialization
83e608d 
* Add _PyWarnings_InitState() which only initializes the _warnings
  module state (tstate->interp->warnings) without creating a module
  object
* Py_InitializeFromConfig() now calls _PyWarnings_InitState() instead
  of _PyWarnings_Init()
* Rename also private functions of _warnings.c to avoid confusion
  between the public C API and the private C API.
@vstinner
Copy link
Member Author

vstinner commented Mar 2, 2020

@shihai1991: this change should help to prepare _warnings.c for adopting the PEP 489 ;-)

@codecov
Copy link

codecov bot commented Mar 2, 2020

Codecov Report

Merging #18739 into master will decrease coverage by 0.00%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #18739      +/-   ##
==========================================
- Coverage   82.14%   82.13%   -0.01%     
==========================================
  Files        1956     1955       -1     
  Lines      590140   584778    -5362     
  Branches    44488    44489       +1     
==========================================
- Hits       484743   480324    -4419     
+ Misses      95745    94803     -942     
+ Partials     9652     9651       -1
Impacted Files Coverage Δ
Lib/distutils/tests/test_bdist_rpm.py 30.00% <0.00%> (-65.00%) ⬇️
Lib/distutils/command/bdist_rpm.py 7.63% <0.00%> (-56.88%) ⬇️
Modules/_decimal/libmpdec/umodarith.h 80.76% <0.00%> (-19.24%) ⬇️
Lib/test/test_urllib2net.py 76.92% <0.00%> (-13.85%) ⬇️
Lib/test/test_smtpnet.py 78.57% <0.00%> (-7.15%) ⬇️
Lib/ftplib.py 63.85% <0.00%> (-6.06%) ⬇️
Lib/test/test_ftplib.py 87.11% <0.00%> (-4.72%) ⬇️
Tools/scripts/db2pickle.py 17.82% <0.00%> (-3.97%) ⬇️
Tools/scripts/pickle2db.py 16.98% <0.00%> (-3.78%) ⬇️
Lib/test/test_socket.py 71.94% <0.00%> (-3.77%) ⬇️
... and 332 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 28d0bca...83e608d. Read the comment docs.

@shihai1991
Copy link
Member

@shihai1991: this change should help to prepare _warnings.c for adopting the PEP 489 ;-)

Wow, victor. Rapid response

@vstinner vstinner merged commit 66b7973 into python:master Mar 2, 2020
@vstinner vstinner deleted the warnings_init branch March 2, 2020 14:02
@vstinner vstinner mentioned this pull request Mar 2, 2020
Closed
shihai1991
shihai1991 reviewed Mar 2, 2020
View reviewed changes
Copy link
Member

@shihai1991 shihai1991 left a comment

Choose a reason for hiding this comment

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

oh, I forgot submit a comment~

Python/_warnings.c
_PyWarnings_InitState(PyThreadState *tstate)
{
if (warnings_init_state(&tstate->interp->warnings) < 0) {
return _PyStatus_ERR("can't initialize warnings");
Copy link
Member

Choose a reason for hiding this comment

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

How about can't initialize warnings state?

@vstinner
Copy link
Member Author

vstinner commented Mar 2, 2020

oh, I forgot submit a comment~

Oh, I already merged my PR. Well, I don't think that it's worth it to change the error message. It should never occur in practice :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@shihai1991 shihai1991 shihai1991 left review comments

Assignees
No one assigned
Labels
skip news
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@vstinner @shihai1991 @the-knights-who-say-ni @bedevere-bot