Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork860
Explain how to add an extension module#1350
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 ourterms of service andprivacy 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
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Thank you Hugo! do you have comments on the content itself, or parts that were not clear enough? |
Content looks okay to me, but let's wait for someone who is more familiar with adding extension modules to CPython to also review. By the way, we don't need to add |
Yes, that's what I observed afterwards. I thought thatmaybe there would be Bedevere that would update the issue automatically but in the end I needed to add the "Fix #..." myself. |
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
picnixz commentedJul 14, 2024 • 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.
I was reading what I wrote and I forgot something about the configure script and bootstrapping so I'll update the tutorial. EDIT: won't have time today anymore! |
- use distinct names for files to highlight differences in configurations- be more precise on the terminology of an extension module- explain how to build a required module (always built-in) vs an optional module (built-in or dynamic)- address Ezio's review
And that |
Uh oh!
There was an error while loading.Please reload this page.
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 mostly looking good, just some comments inline about header files and naming conventions.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Oh, TIL! I never looked in that corner of the code base (yet) :) |
It may make sense to separate out some concepts, such as the limited API, into how-to guides instead of baking them into the tutorial. |
@vstinner has been converting modules to limited API, maybe he's a better person to argue that it should be in the initial example, as a default to start with :)
|
picnixz commentedJul 20, 2024 • 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.
Thank you for the explanation on the Limited API! IIUC:
Ideally, I don't want to duplicate the explanation on the macros themselves (I don't remember where it is) but I can a small paragraph summarizing the above? What remains to add on the page is (in separate PRs):
|
The limited API isdocumented publicly, but Py_BUILD_CORE_MODULE is internal. I think this would be a good place to mention it, since it's only useful for core modules. But, it doesn't need much more than that bullet point. |
I don't really want to add it in the main section because it could disrupt the reading flow. So I think I can add something in the Troubleshooting section when the compiler complains with Actually, there is this page:https://docs.python.org/3/using/configure.html#c-extensions which explains somewhat those macros. Should it be moved to the devguide? it also explains the difference with the (By the way, I don't like the naming "module" and "built-in" and I was confused a lot at first! I would have preferred something like "static" and "shared" because it's much more explanatory... but I think we cannot change this =/ (unless there are some other conditions I am not aware of)). |
Yes, I think mentioning
Maybe some of the confusion remains? IMO, the distinction isn't all that important, and it could moved all the way down to “Configuring the CPython project”, with The reasons for putting a module
If you're adding your first module, these aren't very likely to apply. A similar special case, then, is modules thatmust be built as shared libraries -- that's mostly modules that test the loading of shared libraries. |
I would prefer to keep the doc in the configure docs. You can duplicate the doc in the devguide if you want. |
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.
Nice work! LGTM.
No.... the confusing is because I'm tired and I forgot about the I'll add the paragraph on the macros. I'm waiting forpython/cpython#122544 to be decided because the troubleshooting section will need to be updated (and I'm taking holidays next week so it's better to wait for the correct image to be chosen before merging (it saves us 1 PR if we merge it before)). |
- Add details on `Py_BUILD_CORE_*` macros- Add tips for `Py_LIMITED_API`
…rial# Conflicts:#conf.py
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.
Overall, fantastic job@picnixz. I've made several suggestions to help improve the flow and comprehension of the example. Thank you!
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
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.
Thank you@picnixz for a helpful addition to the devguide. You did a lovely job with this. I'm going to merge this and any further edits can be made in future PRs. 💐 ☀️
3070b7c
intopython:mainUh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
This is a first proposal for explaining how to add an extension module. Feel free to correct me, tell me that it's too verbose or some places need more information!
Fixes#317
📚 Documentation preview 📚:https://cpython-devguide--1350.org.readthedocs.build/developer-workflow/extension-modules/