-
Notifications
You must be signed in to change notification settings - Fork 74
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
Test Python 3.12 support #139
Conversation
@@ -224,7 +224,7 @@ static void _DebugPrintObjects(unsigned int arg_count, ...) | |||
|
|||
int | |||
IS_SUSPENDED(PyFrameObject *frame) { | |||
#if PY_MAJOR_VERSION >= 3 && PY_MINOR_VERSION == 11 | |||
#if PY_MAJOR_VERSION >= 3 && PY_MINOR_VERSION >= 11 |
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 unlikely to be enough, according to a comment below there are APIs in Python 3.12 to do that now?
Your own CI is blocked by python-greenlet/greenlet#327 |
@sumerc greenlet has issued a pre-release with Python 3.12 support, which fixed yappis's CI. You can see the tests passing here with greenlet 3.0.0a1 and my changes: https://github.com/pquentin/yappi/actions/runs/5440881956/jobs/9894253241. What do you think? Note that GitHub Actions no longer supports Python 2.7, are you ready to drop support? I can do that in a different pull request. |
I submitted the PR #145 to remove support of Python2.7 |
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.
Hi @pquentin
I wrote some minor comments
Cheers
@@ -50,20 +50,22 @@ | |||
'License :: OSI Approved :: MIT License', | |||
'Programming Language :: Python', | |||
'Programming Language :: Python :: 2.7', | |||
'Programming Language :: Python :: 3.5', | |||
'Programming Language :: Python :: 3.6', |
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 think those removals should be included in another PR.
And we should remove those lines [1] and adapt [2] as well.
[1] https://github.com/pquentin/yappi/blob/dff6a39973a643a128338d4dae94c846c0072b5d/run_tests.py#L25C1-L26C34
[2] https://github.com/pquentin/yappi/blob/dff6a39973a643a128338d4dae94c846c0072b5d/run_tests.py#L27C1-L28C47
exclude: | ||
- os: windows-latest | ||
python-version: 2.7 # error: Microsoft Visual C++ 9.0 is required | ||
# - os: macos-latest | ||
# python-version: 3.9 # TODO: getting Illegal instruction somehow | ||
continue-on-error: true |
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 can be remove once the PR rebased on top of #145
Thanks for the review. However this was more a proof of concept and I have a lost interest since then. Feel free to reuse my pull request, it has the same license as the code itself. Thanks again! |
I'm trying to add support for the Python 3.12 betas in elastic/rally#1730, but am hitting this error: