-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
PEP 674: Relationship with the HPy project #2247
Merged
Merged
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -284,6 +284,68 @@ known way to emit a deprecation warning only when a macro is used as a | |||||
l-value, but not when it's used differently (ex: as a r-value). | ||||||
|
||||||
|
||||||
Relationship with the HPy project | ||||||
================================= | ||||||
|
||||||
The HPy project | ||||||
--------------- | ||||||
|
||||||
The hope with the HPy project is indeed to provide a C API that is close | ||||||
to the original API to make porting easy and have it perform as close to | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
the existing API as possible. At the same time, HPy is sufficently | ||||||
removed to be a good "C extension API" (as opposed to a stable subset of | ||||||
the CPython implementation API) that does not leak implementation | ||||||
details. To ensure this latter property is why the HPy project tries to | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
develop everything in parallel for CPython, PyPy, and GraalVM Python. | ||||||
|
||||||
HPy is still evolving very fast. Issues are still being solved while | ||||||
migrating NumPy, have begun adding support for HPy to Cython. Working on | ||||||
pybind11 is starting soon. Tim Felgentreff believes by the time HPy has | ||||||
vstinner marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
these users of the existing C API working, HPy should be in a state | ||||||
where it is generally useful and can be deemed stable enough that | ||||||
further development can follow a more stable process. | ||||||
|
||||||
In the long run the HPy project would like to become a promoted API to | ||||||
write Python C extensions. | ||||||
|
||||||
The HPy project is a good solution for the long term. It has the | ||||||
advantage of being developed outside Python and it doesn't require any C | ||||||
API change. | ||||||
|
||||||
The C API is here is stay for a few more years | ||||||
---------------------------------------------- | ||||||
|
||||||
The first concern about HPy is that right now, HPy is not mature nor | ||||||
widely used, and CPython still has to continue supporting a large amount | ||||||
of C extensions which are not likely to be ported to HPy soon. | ||||||
|
||||||
The second concern is the inability to evolve CPython internals to | ||||||
implement new optimizations, and the inefficient implementation of the | ||||||
current C API in PyPy, GraalPython, etc. Sadly, HPy will only solve | ||||||
these problems when most C extensions will be fully ported to HPy: | ||||||
when it will become reasonable to consider dropping the "legacy" Python | ||||||
C API. | ||||||
|
||||||
While porting a C extension to HPy can be done incrementally on CPython, | ||||||
it requires to modify a lot of code and takes time. Porting most C | ||||||
extensions to HPy is expected to take a few years. | ||||||
|
||||||
This PEP proposes to make the C API "less bad" by fixing one problem | ||||||
which is clearily identified as causing practical issues: macros used as | ||||||
l-values. This PEP only requires to update a minority of C extensions, | ||||||
and usually only a few lines need to be changed in impacted extensions. | ||||||
|
||||||
For example, numpy 1.22 is made of 307,300 lines of C code, and adapting | ||||||
numpy to the this PEP only modified 11 lines (use Py_SET_TYPE and | ||||||
Py_SET_SIZE) and adding 4 lines (to define Py_SET_TYPE and Py_SET_SIZE | ||||||
for Python 3.8 and older). Porting numpy to HPy is expected to require | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
modifying more lines than that. | ||||||
|
||||||
Right now, it's hard to bet which approach is the best: fixing the | ||||||
current C API, or focusing on HPy. It would be risky to only focus on | ||||||
HPy. | ||||||
|
||||||
|
||||||
Rejected Idea: Leave the macros as they are | ||||||
=========================================== | ||||||
|
||||||
|
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.
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.