-
Notifications
You must be signed in to change notification settings - Fork 2.2k
fix: support Python 3.14 #5646
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
fix: support Python 3.14 #5646
Conversation
|
The tests/test_operator_overloading.py failure is a trivial fix, but I'm not sure about the other two. They could easily steal a couple hours each to take care of... |
Looks like two errors: I can try bisecting later. |
Yeah, I changed the error message in: python/cpython#132825 You may be able to fix your test by replacing "startswith" with "contains" ("in"). |
The change was documented 1h ago: python/cpython@de28651. |
BTW, we are also running into https://gitlab.kitware.com/cmake/cmake/-/issues/26926 on Windows, CMake is confused and trying to use a free-threaded lib. I've seen that with scikit-build-core, too, in scikit-build/scikit-build-core#1074. |
Bisecting this shows it was broken by python/cpython#123192. I've commented on python/cpython#115776. Edit: opened python/cpython#133912 |
1a879b5
to
63877be
Compare
4ff28e2
to
574a024
Compare
Signed-off-by: Henry Schreiner <[email protected]>
Signed-off-by: Henry Schreiner <[email protected]>
Signed-off-by: Henry Schreiner <[email protected]>
Signed-off-by: Henry Schreiner <[email protected]>
Signed-off-by: Henry Schreiner <[email protected]>
Signed-off-by: Henry Schreiner <[email protected]>
0e89da3
to
2cd17ba
Compare
tests/test_multiple_interpreters.py
Outdated
if sys.version_info >= (3, 15): | ||
import interpreters | ||
elif sys.version_info >= (3, 13): | ||
elif sys.version_info >= (3, 14): | ||
import _interpreters as interpreters |
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.
Looks like they didn't do this rename in 3.14, so it should be _interpreters
for 3.13 and 3.14. Maybe just get rid of the 3.15 and leave it as :
if sys.version_info >= (3, 13):
import _interpreters as interpreters
Or future proof it as:
if sys.version_info >= (3, 13):
try:
import interpreters
except ImportError:
import _interpreters as interpreters
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 there will be a pypi package for 3.14 eventually (though I think it was supposed to come to 3.13 originally, so who knows...), but let's just leave it as <3.15
for now.
Signed-off-by: Henry Schreiner <[email protected]>
Signed-off-by: Henry Schreiner <[email protected]>
Signed-off-by: Henry Schreiner <[email protected]>
Is |
It's not Windows specific, all of them report:
And |
Signed-off-by: Henry Schreiner <[email protected]>
7d3aefb
to
1f2e5d6
Compare
Can be reproduced in pure Python on 3.14t: >>> import sys
>>> class Thing:
... pass
>>> sys._is_immortal(Thing)
False
>>> sys.getrefcount(Thing)
1152921504606846980 Let's just ignore large (30 bits+) values. |
In the 3.14 free threaded build, types, module-level functions, and some other objects use deferred reference counting. They have a large value ( |
There are some more details here: https://peps.python.org/pep-0703/#deferred-reference-counting There's a few things going on here:
Footnotes
|
The test is just making sure pybind11 classes refcount correctly. I think it's fine for it to be checked to be either the correct value or a large value. Unless there's a way to "stop the world" and get the total recount? |
Subinterpreter issue on Windows 3.14t (CC @b-pass):
|
Signed-off-by: Henry Schreiner <[email protected]>
Our single free-threaded test before didn't have support for embedded tests, so that's why this slipped in, I expect. I'm adding all the free-threaded Pythons to the CI. |
In the free threading build for 3.13t and 3.14t, this isn't working: // widget_module did not provide the mod_per_interpreter_gil tag, so it cannot be imported
bool caught = false;
try {
py::module_::import("widget_module");
} catch (pybind11::error_already_set &pe) {
T_REQUIRE(pe.matches(PyExc_ImportError));
std::string msg(pe.what());
T_REQUIRE(msg.find("does not support loading in subinterpreters")
!= std::string::npos);
caught = true;
}
T_REQUIRE(caught);
Though it's clearly printing out:
Is something different about catching exceptions in the free-threading build? macOS (both) and ubuntu 3.13t fail. ubuntu 3.14t and Windows (both) hang. |
I think the issue is concurrent calls to the module init for the embedded modules. I think the below diff would fix it. I can make a separate PR for this if you want. I am working on another PR that includes for switching I don't have any easy way to test this in 3.14t locally, sorry diff --git a/include/pybind11/embed.h b/include/pybind11/embed.h
index a456e80a..b7bd3bec 100644
--- a/include/pybind11/embed.h
+++ b/include/pybind11/embed.h
@@ -41,15 +41,18 @@
#define PYBIND11_EMBEDDED_MODULE(name, variable) \
static ::pybind11::module_::module_def PYBIND11_CONCAT(pybind11_module_def_, name); \
static void PYBIND11_CONCAT(pybind11_init_, name)(::pybind11::module_ &); \
- static PyObject PYBIND11_CONCAT(*pybind11_init_wrapper_, name)() { \
- auto m = ::pybind11::module_::create_extension_module( \
- PYBIND11_TOSTRING(name), nullptr, &PYBIND11_CONCAT(pybind11_module_def_, name)); \
- try { \
- PYBIND11_CONCAT(pybind11_init_, name)(m); \
- return m.ptr(); \
- } \
- PYBIND11_CATCH_INIT_EXCEPTIONS \
- return nullptr; \
+ static PyObject *PYBIND11_CONCAT(pybind11_init_wrapper_, name)() { \
+ static auto result = []() -> PyObject * { \
+ auto m = ::pybind11::module_::create_extension_module( \
+ PYBIND11_TOSTRING(name), nullptr, &PYBIND11_CONCAT(pybind11_module_def_, name)); \
+ try { \
+ PYBIND11_CONCAT(pybind11_init_, name)(m); \
+ return m.ptr(); \
+ } \
+ PYBIND11_CATCH_INIT_EXCEPTIONS \
+ return nullptr; \
+ }(); \
+ return result; \
} \
PYBIND11_EMBEDDED_MODULE_IMPL(name) \
::pybind11::detail::embedded_module PYBIND11_CONCAT(pybind11_module_, name)( \ |
Signed-off-by: Henry Schreiner <[email protected]>
I'm low on battery, so I'll drop the free threaded ci and that can be a separate PR. I can try things locally normally but that will finish my battery. ;) I tried your patch but it looks like it's failing. Sounds good. |
This reverts commit c4226a0.
Signed-off-by: Henry Schreiner <[email protected]>
@@ -86,7 +86,7 @@ def test_dependent_subinterpreters(): | |||
|
|||
sys.path.append(".") | |||
|
|||
if sys.version_info >= (3, 14): | |||
if sys.version_info >= (3, 15): |
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.
Is this final for 3.14, or could this still change before the 3.14.0 release?
If the latter: maybe add a comment?
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.
It will continue to work. I think intepreters
is supposed to be released on PyPI during the 3.14 lifecycle.
Signed-off-by: Henry Schreiner <[email protected]>
Description
Let's try beta 1!
Suggested changelog entry: