Skip to content
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

Python 3.13 support #16851

Merged
merged 3 commits into from
Nov 7, 2024
Merged

Python 3.13 support #16851

merged 3 commits into from
Nov 7, 2024

Conversation

guitargeek
Copy link
Contributor

@guitargeek guitargeek commented Nov 7, 2024

  1. Sync with upstream cppyy, mostly to avoid warnings when building with Python 3.13.
  2. Use GetOptional family of C Python API where necessary

A regression in ROOT 6.32 was unearthed in the process: #16852.

@guitargeek
Copy link
Contributor Author

guitargeek commented Nov 7, 2024

The Fedora 41 is temporarily added to the CI to get test coverage for Python 3.13, but once the PR is shown to do what it should, the CI commit will be removed.

@guitargeek guitargeek changed the title [CPyCppyy] Don't attempt to expose protected data members in dispatcher Python 3.13 support Nov 7, 2024
guitargeek and others added 2 commits November 7, 2024 12:33
With Python 3.13, some lookup methods like `PyMapping_GetItemString` and
`PyObject_GetAttrString` became more strict. They are now always
throwing an exception in case the attribute is not found.

To make these optional lookups work again, the `GetOptional` family of
functions needs to be used.

See:
  * https://docs.python.org/3/c-api/object.html#c.PyObject_GetOptionalAttrString
  * https://docs.python.org/3/c-api/mapping.html#c.PyMapping_GetOptionalItemString
@guitargeek
Copy link
Contributor Author

The failures on Fedora are all gone! Except for the expected one. Hence, I'll leave the Fedora 41 commit in this PR and restart the tests one more time.

@guitargeek guitargeek closed this Nov 7, 2024
@guitargeek guitargeek reopened this Nov 7, 2024
Copy link

github-actions bot commented Nov 7, 2024

Test Results

    13 files      13 suites   2d 10h 21m 21s ⏱️
 2 629 tests  2 628 ✅ 0 💤  1 ❌
33 654 runs  33 641 ✅ 0 💤 13 ❌

For more details on these failures, see this check.

Results for commit a0b526c.

Copy link
Member

@vepadulano vepadulano left a comment

Choose a reason for hiding this comment

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

Thank you! Feel free to merge once the CI is green

@guitargeek guitargeek merged commit 2208a45 into root-project:master Nov 7, 2024
20 of 43 checks passed
@guitargeek guitargeek deleted the py313 branch November 7, 2024 20:13
Comment on lines -217 to +225
PyObject _CPyCppyy_NullPtrStruct = {_PyObject_EXTRA_INIT
// In 3.12.0-beta this field was changed from a ssize_t to a union
#if PY_VERSION_HEX >= 0x30c00b1
{1},
#else
1,
#endif
&PyNullPtr_t_Type};
PyObject _CPyCppyy_NullPtrStruct = {
_PyObject_EXTRA_INIT
1, &PyNullPtr_t_Type
};

PyObject _CPyCppyy_DefaultStruct = {_PyObject_EXTRA_INIT
// In 3.12.0-beta this field was changed from a ssize_t to a union
#if PY_VERSION_HEX >= 0x30c00b1
{1},
#else
1,
#endif
&PyDefault_t_Type};
PyObject _CPyCppyy_DefaultStruct = {
_PyObject_EXTRA_INIT
1, &PyDefault_t_Type
};
Copy link
Member

Choose a reason for hiding this comment

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

This removes the changes done by @silverweed to avoid compiler warnings...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah! I have to re-introduce it then, also adding a patch file. Then I won't accidentally revert it anymore

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants