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

Use PyMapping_GetOptionalItemString where necessary with Python 3.13 #44

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

guitargeek
Copy link
Contributor

@guitargeek guitargeek commented Dec 17, 2024

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:

This is the upstream version of the following ROOT commit:

Related to the following cppyy issue, although it might be better to close that issue by fixing the underlying problem:

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

This is the upstream version of the following ROOT commit:

  * root-project/root@e78450dc45ed868b7a52a0
@wlav
Copy link
Owner

wlav commented Dec 17, 2024

Interesting, since even with py13, wlav/cppyy#273 doesn't fail for me.

@guitargeek
Copy link
Contributor Author

Yes, I've read that. Maybe my reproducer in the issue is not complete. I'll investigate this further.

@wlav
Copy link
Owner

wlav commented Dec 17, 2024

As for the PR, I'm much rather use a more descriptive name and put a compatibility function in CPyCppyy.h. I'll do that later: I'm going to remove all Python2 stuff, finally. My last place to test it was an old Mac, but that one has other issues now, so the most recent release didn't have code verified on it.

This graph shows that py3.7 is the oldest I need to care about.

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

Successfully merging this pull request may close these issues.

2 participants