search

pybind / pybind11

Seamless operability between C++11 and Python
https://pybind11.readthedocs.io/
Other
15.1k stars 2.05k forks source link

fix: Use PyObject_VisitManagedDict() of Python 3.13 #4973

Closed vstinner closed 6 months ago

vstinner commented 6 months ago

Use PyObject_VisitManagedDict() and PyObject_ClearManagedDict() in pybind11_traverse() and pybind11_clear() on Python 3.13 and newer.

Description

Suggested changelog entry:

Use ``PyObject_VisitManagedDict()`` and ``PyObject_ClearManagedDict()`` on Python 3.13 and newer.
vstinner commented 6 months ago

cc @rwgk

vstinner commented 6 months ago

I added Python 3.13 to .github/workflows/ci.yml to test the change.

vstinner commented 6 months ago

Oh, "🐍 3.13 • ubuntu-20.04 • x64" failed with:

ERROR: Could not find a version that satisfies the requirement numpy~=1.26.0 (from versions: none)
ERROR: No matching distribution found for numpy~=1.26.0
vstinner commented 6 months ago

@henryiii: Oh thanks for your fix, the three CI jobs running on Python 3.13 succeeded with your change.

Should I add Python 3.13 to the CI in a separated PR? Or is it ok to add it in the same PR? As you wish.

henryiii commented 6 months ago

Any breaking changes in 3.13 alphas/betas will also break our main workflow, which isn't ideal. In the past we've had an upstream job for this. Actually, it looks like it's still there, just running 3.12 still. Let me see if I can switch to that.

(Making the CI change in this PR is fine, btw)

rwgk commented 6 months ago

The 🐍 3.12 • macos-latest • x64 workflow was stuck for 3+ hours. I canceled it and then triggered a rerun.

vstinner commented 6 months ago

Nice, thanks for the help with the CI change.

rwgk commented 6 months ago

FWIW — Maybe for later:

upstream.yml only exercises one specific platform (ubuntu).

I think adding 3.13 testing in ci.yml would be best in general, because then we'd get a lot more coverage.

It would also have value to run 3.13 testing by default, with an option to disable it via a Label when we hit a time window in which 3.13 testing is broken because of a root cause that's in core Python (rather than pybind11). That way we wouldn't have to remember to test with 3.13, and we'd catch new core Python issues more-or-less immediately. The only annoyance would be that we'd have to use the Label until the issue is fixed.

What I don't know: Is there an easy way to exclude a matrix element (in ci.yml) based on a Label?