FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Add PyCode_GetCode() function by vstinner · Pull Request #34 · python/pythoncapi-compat · GitHub

/ pythoncapi-compat Public

Add PyCode_GetCode() function - #34

Merged
vstinner merged 2 commits into
python:mainfrom
vstinner:code_getcode
May 3, 2022
ConversationCommits (2)ChecksFiles changed
Merged

Add PyCode_GetCode() function#34
vstinner merged 2 commits into
python:mainfrom
vstinner:code_getcode

Conversation

vstinner commented May 3, 2022

Copy link
Copy Markdown
Member

No description provided.

vstinner commented May 3, 2022 •
edited
Loading

Copy link
Copy Markdown
Member Author

Wait until python/cpython#92168 is merged.

vstinner marked this pull request as draft May 3, 2022 11:37
vstinner mentioned this pull request May 3, 2022
Merged
vstinner marked this pull request as ready for review May 3, 2022 13:45
vstinner merged commit 85d1bd8 into python:main May 3, 2022
vstinner deleted the code_getcode branch May 3, 2022 13:49
Fidget-Spinner reviewed May 3, 2022
View reviewed changes
Comment thread docs/api.rst
Python 3.11
-----------

.. c:function:: PyObject* PyCode_GetCode(PyObject *code)

Fidget-Spinner May 3, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Nit: PyCodeObject *code not PyObject *code :)

vstinner May 3, 2022

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Fixed in commit bed155e.

Fidget-Spinner reviewed May 3, 2022
View reviewed changes
Comment thread pythoncapi_compat.h


// gh-92154 added PyCode_GetCode() to Python 3.11.0b1
#if PY_VERSION_HEX < 0x030B00B1 && !defined(PYPY_VERSION)

Fidget-Spinner May 3, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Not sure if it matters, but this code will fail on one version: 3.11a7. In that version, co_code was removed, but there is no PyCode_GetCode.

I don't know what you should do here. Maybe exclude that version?

erlend-aasland May 3, 2022

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Perhaps #error with an explanatory message.

vstinner May 3, 2022

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I don't know what you should do here. Maybe exclude that version?

Alpha versions of Python 3.11 are not supported:

pythoncapi-compat/tests/test_pythoncapi_compat.py

Lines 152 to 157 in c9764f5

# Implementing PyFrame_GetLocals() and PyCode_GetCode() require the
# internal C API in Python 3.11 alpha versions
if 0x30b0000 <= sys.hexversion < 0x30b00b1:
version = sys.version.split()[0]
print("SKIP TESTS: Python %s is not supported" % version)
return

It's a deliberate choice to not pollute pythoncapi_compat.h with ugly code to tons of compatibility layers, whereas there are few users of Python alpha versions.

Perhaps #error with an explanatory message.

What I suggested to users who want to upgrade pythoncapi_compat.h before Python 3.11 beta1 is to copy the file and then remove the functions which cause compilation errors. It's being discussed at: #32

erlend-aasland May 3, 2022

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Ok, makes sense.

vstinner May 3, 2022

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I'm not against adding an #error if you consider that it will help users impacted by this issue.

Python 3.11 beta1 is scheduled in a few days.

Python 3.11 breaks tons of C API: https://docs.python.org/dev/whatsnew/3.11.html#c-api-changes

In the past, I tried first to add getter functions in one Python version, and only change the structure in the following Python version. Well, the optimization work broke many structures (PyThreadState, PyFrameObject, PyCodeObject). But after beta1, the API should be stable.

This file contains hidden or 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
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

erlend-aasland erlend-aasland left review comments
+1 more reviewer
Fidget-Spinner Fidget-Spinner left review comments
Reviewers whose approvals may not affect merge requirements

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

Back | FazBrowse Home | New Git URL