-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
PEP 793: PyModExport: A new entry point for C extension modules #4435
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
Conversation
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.
Thanks!
General points:
- I think the PEP would benefit from more examples. Perhaps a full example of a module only using
PyModExport_*
, an example on porting, and an example on using both export hooks at once, e.g. for a module supporting Python 3.11--3.15? - The text is very heavy on parenthetical asides. I've suggested removing some, but in general I think where full sentences are in brackets, the text would be stronger by properly incorporating the thought and removing the brackets.
- Please replace typographical quotation marks with
"
/'
, Sphinx/Docutils insert them automatically. - Naming (sorry!): Currently,
PyMod*
is only used byPyModInitFunction
. WouldPyModuleExport
be considered?
A
Co-authored-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com> Co-authored-by: Hood Chatham <roberthoodchatham@gmail.com> Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com>
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 for the reviews! I went through some of the comments, will get to the rest later.
kid is waking up, I left some replies terse; please imagine a most friendly & respectful tone :)
I think the PEP would benefit from more examples. Perhaps a full example of a module only using PyModExport_*, an example on porting, and an example on using both export hooks at once, e.g. for a module supporting Python 3.11--3.15?
Agree, but these should end up in docs and I don't want people reading them here after acceptance. I'll add them to the reference implementation and point the PEP there.
The text is very heavy on parenthetical asides. I've suggested removing some, but in general I think where full sentences are in brackets, the text would be stronger by properly incorporating the thought and removing the brackets.
I use them to separate the hard spec from “mere“ implications/explanations. They are very helpful for me, and I like to think they're not too distracting for the reader.
Please replace typographical quotation marks with "/', Sphinx/Docutils insert them automatically.
I don't see a reason to do that. Any future editor is free to use "
instead, it's not like Python identifiers which you need to repeat exactly somewhere else.
Naming (sorry!): Currently, PyMod* is only used by PyModInitFunction. Would PyModuleExport be considered?
That's a point for the discussion :)
(Yes, I considered it, but my conclusion is not strong enough for a Rejected Ideas entry and the question is not important enough for Open Questions. I'm literally waiting for the discussion thread here.)
The docs are in flux, so I added an example and porting guide to the PEP. |
Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com>
Thank you for the reviews! |
Basic requirements (all PEP Types)
pep-NNNN.rst
), PR title (PEP 123: <Title of PEP>
) andPEP
headerAuthor
orSponsor
, and formally confirmed their approvalAuthor
,Status
(Draft
),Type
andCreated
headers filled out correctlyPEP-Delegate
,Topic
,Requires
andReplaces
headers completed if appropriate.github/CODEOWNERS
for the PEPStandards Track requirements
Python-Version
set to valid (pre-beta) future Python version, if relevantDiscussions-To
andPost-History
📚 Documentation preview 📚: https://pep-previews--4435.org.readthedocs.build/pep-0793/