Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 33.9k
gh-107298: Document doesn't link to PyAPI_DATA()#109236
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
Uh oh!
There was an error while loading. Please reload this page.
Conversation
vstinner commented Sep 10, 2023 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
vstinner commented Sep 10, 2023
vstinner left a comment
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.
Oh! There is still a warning in Doc/whatsnew/2.3.rst: c:macro reference target not found: PyMODINIT_FUNC.
I will fix it.
cbd269f to 553fdd0Compare
serhiy-storchaka left a comment
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.
PyMODINIT_FUNC is purposed to be used in the user code. It is even used in examples.
553fdd0 to 5eba491Comparevstinner commented Sep 11, 2023
Oh right. I fixed my PR:
Please review it again. @hugovk: I added "the" in the added doc ;-) |
5eba491 to 5847580CompareDoc/c-api/intro.rst Outdated
| type is :c:expr:`PyObject*`. The macro should only be used in the Python C | ||
| API. |
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.
"The macro should only be used in the Python C API." -- I think that it is wrong.
This description is not clear, and does not help. The use of PyMODINIT_FUNC is actually described in Doc/extending/extending.rst:
This structure, in turn, must be passed to the interpreter in the module's initialization function. The initialization function must be named :c:func:`!PyInit_name`, where *name* is the name of the module, and should be the only non-\ ``static`` item defined in the module file:: PyMODINIT_FUNC PyInit_spam(void){ return PyModule_Create(&spammodule); } Note that :c:macro:`PyMODINIT_FUNC` declares the function as ``PyObject *`` return type, declares any special linkage declarations required by the platform, and for C++ declares the function as ``extern "C"``.It would be better if PyMODINIT_FUNC links would refer to that text. Or at least make a reference to it from the declaration in Doc/c-api/intro.rst.
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.
Ok, I copied this doc to Doc/c-api/intro.rst.
vstinner commented Sep 13, 2023
@serhiy-storchaka: I completed PyMODINIT_FUNC doc. Would you mind to review my doc change again? |
vstinner commented Sep 13, 2023
Oops sorry, I tested a Git tool and it messed up my PR :-( I fixed my PR. |
Doc/c-api/intro.rst Outdated
| declarations required by the platform, and for C++ declares the function as | ||
| ``extern "C"``. | ||
| The initialization function must be named :c:func:`!PyInit_name`, where |
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.
Maybe write :samp:`PyInit_{name}`? It renders the whole name as a code, but name additionally is in italic, as a variable part.
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 never saw :samp:, let me try.
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.
vstinner commented Sep 14, 2023
Merged, thanks for the review @serhiy-storchaka. |
vstinner commented Sep 14, 2023
Oh, and thanks @hugovk for the review as well :-) |
miss-islington commented Sep 27, 2023
Thanks @vstinner for the PR 🌮🎉.. I'm working now to backport this PR to: 3.11. |
miss-islington commented Sep 27, 2023
Thanks @vstinner for the PR 🌮🎉.. I'm working now to backport this PR to: 3.12. |
miss-islington commented Sep 27, 2023
Sorry, @vstinner, I could not cleanly backport this to |
Document PyMODINIT_FUNC macro. Remove links to PyAPI_FUNC() and PyAPI_DATA() macros since they are not documented. These macros should only be used to define the Python C API. They should not be used outside Python code base. (cherry picked from commit d7a27e5) Co-authored-by: Victor Stinner <vstinner@python.org>
GH-109947 is a backport of this pull request to the 3.12 branch. |
GH-109948 is a backport of this pull request to the 3.11 branch. |
gh-107298: Document PyMODINIT_FUNC macro (#109236) Document PyMODINIT_FUNC macro. Remove links to PyAPI_FUNC() and PyAPI_DATA() macros since they are not documented. These macros should only be used to define the Python C API. They should not be used outside Python code base. (cherry picked from commit d7a27e5)
gh-107298: Document PyMODINIT_FUNC macro (GH-109236) Document PyMODINIT_FUNC macro. Remove links to PyAPI_FUNC() and PyAPI_DATA() macros since they are not documented. These macros should only be used to define the Python C API. They should not be used outside Python code base. (cherry picked from commit d7a27e5) Co-authored-by: Victor Stinner <vstinner@python.org>

Remove links to PyAPI_FUNC() and PyAPI_DATA() macros since they are
not documented. These macros should only be used to define the Python
C API. They should not be used outside Python code base.
Document PyMODINIT_FUNC macro.
📚 Documentation preview 📚: https://cpython-previews--109236.org.readthedocs.build/