Skip to content

gh-105922: Add PyImport_AddModuleRef() function - #105923

Merged
vstinner merged 1 commit into
python:mainfrom
vstinner:pyimport_addmoduleref
Jun 20, 2023
Merged

gh-105922: Add PyImport_AddModuleRef() function#105923
vstinner merged 1 commit into
python:mainfrom
vstinner:pyimport_addmoduleref

Conversation

@vstinner

@vstinnervstinner commented Jun 19, 2023•
edited
Loading

Copy link
Copy Markdown
Member
  • Add tests on PyImport_AddModuleRef(), PyImport_AddModule() and
    PyImport_AddModuleObject().
  • pythonrun.c: Replace Py_XNewRef(PyImport_AddModule(name)) with
    PyImport_AddModuleRef(name).

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

@bedevere-botbedevere-bot mentioned this pull request Jun 19, 2023
Closed
erlend-aasland
erlend-aasland reviewed Jun 19, 2023
View reviewed changes

@erlend-aaslanderlend-aasland left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice; I left some comments.

Too bad we cannot run the ref leak bots now; there's already a ref leak in test_import and test_peg_generator that has not been resolved :(

Comment threadInclude/import.h
Comment threadMisc/NEWS.d/next/C API/2023-06-19-20-02-16.gh-issue-105922.o4T6wO.rst Outdated
Comment threadPython/import.c Outdated
Comment threadPython/import.c Outdated
Comment threadPython/pythonrun.c Outdated
@vstinner
vstinnerforce-pushed the pyimport_addmoduleref branch from 631c02c to bf2a756CompareJune 19, 2023 23:08
@vstinner

vstinner commented Jun 19, 2023

Copy link
Copy Markdown
MemberAuthor

I updated my PR:

  • Rebased on the merged pythonrunc.c refactoring: commit a5c2ad0
  • Don't deprecate PyImport_AddModule() and PyImport_AddModuleObject() in this PR anymore: I prefer to handle this in a separated PR.
  • Fix pythonrun.c
  • Use PyImport_AddModuleRef() for _frozen_importlib

erlend-aasland
erlend-aasland approved these changes Jun 19, 2023
View reviewed changes

@erlend-aaslanderlend-aasland left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Small typo nit. Otherwise looks good to me.

Comment threadDoc/whatsnew/3.13.rst Outdated
Comment threadMisc/NEWS.d/next/C API/2023-06-19-20-02-16.gh-issue-105922.o4T6wO.rst Outdated
@vstinner
pythongh-105922: Add PyImport_AddModuleRef() function
f3a223d
* Add tests on PyImport_AddModuleRef(), PyImport_AddModule() and
PyImport_AddModuleObject().
* pythonrun.c: Replace Py_XNewRef(PyImport_AddModule(name)) with
PyImport_AddModuleRef(name).
@vstinner

vstinner commented Jun 20, 2023

Copy link
Copy Markdown
MemberAuthor

I fixed the typo: similar than => similar to.

@vstinner
vstinnerforce-pushed the pyimport_addmoduleref branch from 22c5f8c to f3a223dCompareJune 20, 2023 00:36
@vstinner
vstinner merged commit 03f1a13 into python:mainJun 20, 2023
@vstinner
vstinner deleted the pyimport_addmoduleref branch June 20, 2023 06:48
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@erlend-aaslanderlend-aaslanderlend-aasland approved these changes
@encukouencukouAwaiting requested review from encukouencukou is a code owner
@brettcannonbrettcannonAwaiting requested review from brettcannonbrettcannon is a code owner
@ericsnowcurrentlyericsnowcurrentlyAwaiting requested review from ericsnowcurrentlyericsnowcurrently is a code owner
@ncoghlanncoghlanAwaiting requested review from ncoghlanncoghlan is a code owner
@warsawwarsawAwaiting requested review from warsawwarsaw is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@vstinner@erlend-aasland@bedevere-bot