GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-134160: Use multi-phase init in documentation examples by neonene · Pull Request #134296 · python/cpython

Conversation

@neonene

@neonene neonene commented May 19, 2025 •
edited by github-actions Bot
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

Replaces PyModule_Create() with PyModuleDef_Init().

Static types still remain unchanged in the doc.

  • Issue: Docs Examples Focus on Outdated Single-Phase Init Modules and Static Types #134160

📚 Documentation preview 📚: https://cpython-previews--134296.org.readthedocs.build/

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@neonene
Update docs
8d8b366
@bedevere-app bedevere-app Bot mentioned this pull request May 19, 2025
Docs Examples Focus on Outdated Single-Phase Init Modules and Static Types #134160
Open
@bedevere-app bedevere-app Bot added the awaiting review label May 19, 2025
@neonene

neonene commented May 20, 2025

Copy link
Copy Markdown
Contributor Author

cc @erlend-aasland @encukou Just in case you're interested.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@neonene
Don't change an unrelated example
88c5f3f
@AA-Turner AA-Turner changed the title gh-134160: Docs: Ensure single-phase init is no longer used in examples gh-134160: Use multi-phase init in documentation examples May 21, 2025
@AA-Turner AA-Turner added the docs Documentation in the Doc dir label May 21, 2025
@github-project-automation github-project-automation Bot added this to Docs PRs May 21, 2025
@github-project-automation github-project-automation Bot moved this to Todo in Docs PRs May 21, 2025
@AA-Turner AA-Turner added needs backport to 3.13 only security fixes needs backport to 3.14 bugs and security fixes labels May 21, 2025
@AA-Turner
AA-Turner requested a review from ericsnowcurrently May 21, 2025 00:01
AA-Turner
AA-Turner reviewed May 21, 2025
View reviewed changes
Comment thread Doc/c-api/intro.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Doc/extending/extending.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Doc/extending/extending.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Doc/extending/extending.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Doc/extending/extending.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Doc/extending/extending.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Doc/extending/extending.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Doc/extending/extending.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Doc/extending/extending.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Doc/extending/extending.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
neonene and others added 11 commits May 21, 2025 18:43
@neonene @AA-Turner
Apply suggestions from code review
a4ade5c 
Co-authored-by: Adam Turner <[email protected]>
@neonene
ditto
d6c42e2
@neonene
Suggested module names
97eae13
@neonene
Avoid modernization
8ae6bae
@neonene
typo
bc78288
@neonene
Correction: "error"
c3ab022
@neonene
Rephrase warning
d65303c
@neonene
keywdarg_module
aec6781
@neonene @AA-Turner
Update Doc/extending/extending.rst
7dfd5ad 
Co-authored-by: Adam Turner <[email protected]>
@neonene
Indicate xxlimited
f7c72dd
@neonene
Add seealso to index
c84638c
AA-Turner
AA-Turner reviewed May 22, 2025
View reviewed changes
Comment thread Doc/extending/embedding.rst Outdated
Comment on lines +254 to +258
static PyModuleDef_Slot emb_module_slots[] = {
{Py_mod_multiple_interpreters, Py_MOD_PER_INTERPRETER_GIL_SUPPORTED},
{Py_mod_gil, Py_MOD_GIL_NOT_USED},
{0, NULL}
};

@AA-Turner AA-Turner May 22, 2025 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Having done trial conversions of NumPy to multi-phase, I think we should include the relevant PY_VERSION_HEX checks. Both Py_mod_multiple_interpreters and Py_mod_gil were added very recently.

If we agree on adding these checks, we should add them for all relevant PyModuleDef_Slot structs. It is verbose, but aids in the copy-and-paste scenario.

Suggested change
static PyModuleDef_Slot emb_module_slots[] = {
{Py_mod_multiple_interpreters, Py_MOD_PER_INTERPRETER_GIL_SUPPORTED},
{Py_mod_gil, Py_MOD_GIL_NOT_USED},
{0, NULL}
};
static PyModuleDef_Slot emb_module_slots[] = {
#if PY_VERSION_HEX >= 0x030C00F0 // Python 3.12+
// signal that this module can be imported in isolated subinterpreters
{Py_mod_multiple_interpreters, Py_MOD_PER_INTERPRETER_GIL_SUPPORTED},
#endif
#if PY_VERSION_HEX >= 0x030D00F0 // Python 3.13+
// signal that this module supports running without an active GIL
{Py_mod_gil, Py_MOD_GIL_NOT_USED},
#endif
{0, NULL}
};

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@AA-Turner AA-Turner May 22, 2025

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Alternative:

Suggested change
static PyModuleDef_Slot emb_module_slots[] = {
{Py_mod_multiple_interpreters, Py_MOD_PER_INTERPRETER_GIL_SUPPORTED},
{Py_mod_gil, Py_MOD_GIL_NOT_USED},
{0, NULL}
};
static PyModuleDef_Slot emb_module_slots[] = {
#ifdef Py_mod_multiple_interpreters
// signal that this module can be imported in isolated subinterpreters
{Py_mod_multiple_interpreters, Py_MOD_PER_INTERPRETER_GIL_SUPPORTED},
#endif
#ifdef Py_mod_gil
// signal that this module supports running without an active GIL
{Py_mod_gil, Py_MOD_GIL_NOT_USED},
#endif
{0, NULL}
};

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
neonene reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

@encukou encukou May 22, 2025

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Can this PR omit these slots entirely?

IMO, they should be introduced later, after you compile your first module, along with a longer discussion about isolated state & thread safety.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
AA-Turner reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

@AA-Turner AA-Turner May 22, 2025

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Agree better to keep simple. @neonene perhaps remove the gil and interpreter slots from where we've added them in this PR unless they were already present?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@neonene neonene May 22, 2025

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Removed, but not all. I can complete.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@AA-Turner AA-Turner added the skip news label May 22, 2025
@AA-Turner

AA-Turner commented May 22, 2025

Copy link
Copy Markdown
Member

Other places to update:

  • https://cpython-previews--134296.org.readthedocs.build/en/134296/c-api/allocation.html: PyModule_Create() To allocate and create extension modules. Should point to multi-phase modules & note PyModule_Create is for static.
  • https://cpython-previews--134296.org.readthedocs.build/en/134296/howto/free-threading-extensions.html#single-phase-initialization Should suggest adopting multi-phase with no isolation before describing the PyUnstable function, perhaps link to the porting guide
  • https://cpython-previews--134296.org.readthedocs.build/en/134296/c-api/module.html#initializing-c-modules Mention multi-phase before PyModule_Create
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
neonene and others added 3 commits May 22, 2025 16:05
@neonene @AA-Turner
Update Doc/extending/embedding.rst
4e59e88 
Co-authored-by: Adam Turner <[email protected]>
@neonene
ditto (for copy-paste)
d68e3e3
@neonene
Shrink slots
f6465bd
neonene added 3 commits May 23, 2025 07:10
@neonene
Correction: "SpamError"
a83e266
@neonene
Add a Py_mod_multiple_interpreters slot
2d10958
@neonene
Cancel the prvious addition as needless
a28e67c
@neonene

neonene commented May 22, 2025

Copy link
Copy Markdown
Contributor Author

One of my motivation: https://discuss.python.org/t/proposal-officially-deprecate-support-for-legacy-single-phase-init-extension-modules/89262/26
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
AA-Turner added 3 commits May 26, 2025 20:26
@AA-Turner
Merge remote-tracking branch 'upstream/main' into multi
9e6bff5
@AA-Turner
updates
d35a6b4
@AA-Turner
PEP 7
9cb3092
AA-Turner
AA-Turner approved these changes May 26, 2025
View reviewed changes
@bedevere-app bedevere-app Bot added awaiting merge and removed awaiting review labels May 26, 2025
@AA-Turner
revert module state for SpamError
6e5b1c5
@AA-Turner
AA-Turner enabled auto-merge (squash) May 26, 2025 21:39
@AA-Turner
AA-Turner merged commit 96905bd into python:main May 26, 2025
@github-project-automation github-project-automation Bot moved this from Todo to Done in Docs PRs May 26, 2025
@miss-islington-app

miss-islington-app Bot commented May 26, 2025

Copy link
Copy Markdown

Thanks @neonene for the PR, and @AA-Turner for merging it 🌮🎉.. I'm working now to backport this PR to: 3.13, 3.14.
🐍🍒⛏🤖 I'm not a witch! I'm not a witch!
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-app bedevere-app Bot removed the awaiting merge label May 26, 2025
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request May 26, 2025
@neonene @AA-Turner @miss-islington
pythongh-134160: Use multi-phase init in documentation examples (pyth…
f6398e2 
…onGH-134296)

(cherry picked from commit 96905bd)

Co-authored-by: neonene <[email protected]>
Co-authored-by: Adam Turner <[email protected]>
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request May 26, 2025
@neonene @AA-Turner @miss-islington
pythongh-134160: Use multi-phase init in documentation examples (pyth…
0930d33 
…onGH-134296)

(cherry picked from commit 96905bd)

Co-authored-by: neonene <[email protected]>
Co-authored-by: Adam Turner <[email protected]>
@bedevere-app

bedevere-app Bot commented May 26, 2025

Copy link
Copy Markdown

GH-134753 is a backport of this pull request to the 3.14 branch.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-app bedevere-app Bot removed the needs backport to 3.14 bugs and security fixes label May 26, 2025
@bedevere-app

bedevere-app Bot commented May 26, 2025

Copy link
Copy Markdown

GH-134754 is a backport of this pull request to the 3.13 branch.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-app bedevere-app Bot removed the needs backport to 3.13 only security fixes label May 26, 2025
AA-Turner added a commit that referenced this pull request May 26, 2025
@miss-islington @neonene @AA-Turner
[3.14] gh-134160: Use multi-phase init in documentation examples (GH-…
b6e624a 
…134296) (#134753)

gh-134160: Use multi-phase init in documentation examples (GH-134296)
(cherry picked from commit 96905bd)

Co-authored-by: neonene <[email protected]>
Co-authored-by: Adam Turner <[email protected]>
AA-Turner added a commit that referenced this pull request May 26, 2025
@miss-islington @neonene @AA-Turner
[3.13] gh-134160: Use multi-phase init in documentation examples (GH-…
480f02d 
…134296) (#134754)

gh-134160: Use multi-phase init in documentation examples (GH-134296)
(cherry picked from commit 96905bd)

Co-authored-by: neonene <[email protected]>
Co-authored-by: Adam Turner <[email protected]>
@AA-Turner AA-Turner mentioned this pull request May 27, 2025
GH-134160: Prefer multi-phase initialisation in docs #134764
Closed
encukou
encukou reviewed May 27, 2025
View reviewed changes
Comment thread Doc/extending/extending.rst
Comment on lines +206 to +208
You can also define a new exception that is unique to your module.
For this, you can declare a static global object variable at the beginning
of the file::

@encukou encukou May 27, 2025

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

This is not correct for multi-phase initialization, without preventing the exec function from other module instances from overriding this pointer.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@AA-Turner AA-Turner May 27, 2025

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

@encukou I added but then reverted (6e5b1c5) module state for this error type -- should we un-revert that commit? Sorry for my mistake here.

A

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@encukou encukou May 27, 2025

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

I think it's fine -- module state deserves a better explanation a bit later in the tutorial. And the ImportError being added in the next PR is a good thing to show.

The main thing I'd do differently is not backporting to 3.13 -- that's like pushing directly to production. 3.14 is out in a few months; that sounds like a good amount of time for this all to spend in prerelease.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@neonene
neonene deleted the multi branch May 27, 2025 05:48
Pranjal095 pushed a commit to Pranjal095/cpython that referenced this pull request Jul 12, 2025
@neonene @AA-Turner @Pranjal095
pythongh-134160: Use multi-phase init in documentation examples (pyth…
cfecd06 
…on#134296)

Co-authored-by: Adam Turner <[email protected]>
taegyunkim pushed a commit to taegyunkim/cpython that referenced this pull request Aug 4, 2025
@neonene @AA-Turner @taegyunkim
pythongh-134160: Use multi-phase init in documentation examples (pyth…
2ea7d29 
…on#134296)

Co-authored-by: Adam Turner <[email protected]>
gostak-dd pushed a commit to gostak-dd/cpython that referenced this pull request Jun 2, 2026
@neonene @AA-Turner @gostak-dd
pythongh-134160: Use multi-phase init in documentation examples (pyth…
9a68024 
…on#134296)

Co-authored-by: Adam Turner <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@encukou encukou encukou left review comments
@AA-Turner AA-Turner AA-Turner approved these changes
@ericsnowcurrently ericsnowcurrently Awaiting requested review from ericsnowcurrently

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

docs Documentation in the Doc dir skip news

Projects

Docs PRs
Status: Done

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

3 participants

@neonene @AA-Turner @encukou
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.