From ee04df0d027d89deee8bc58c98e80b4d73a21213 Mon Sep 17 00:00:00 2001 From: Tim Ohliger Date: Mon, 14 Apr 2025 04:47:19 +0200 Subject: [PATCH 1/7] Updated STL casters and py::buffer to use collections.abc (#5566) * Updated STL type hints use support collections.abc * Updated array_caster to match numpy/eigen typing.Annotated stlye * Added support for Mapping, Set and Sequence derived from collections.abc. * Fixed merge of typing.SupportsInt in new tests * Integrated collections.abc checks into convertible check functions. * Changed type hint of py::buffer to collections.abc.Buffer * Changed convertible check function names * Added comments to convertible check functions * Removed checks for methods that are already required by the abstract base class * Improved mapping caster test using more compact a1b2c3 variable * Renamed and refactored sequence, mapping and set test classes to reuse implementation * Added tests for mapping and set casters for noconvert mode * Added tests for sequence caster for noconvert mode --- include/pybind11/cast.h | 2 +- include/pybind11/stl.h | 95 ++++++++++------- tests/test_buffers.py | 2 +- tests/test_kwargs_and_defaults.py | 2 +- tests/test_pytypes.py | 2 +- tests/test_stl.cpp | 15 +++ tests/test_stl.py | 169 ++++++++++++++++++++++++++++-- 7 files changed, 237 insertions(+), 50 deletions(-) diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h index 410215244..8952cf094 100644 --- a/include/pybind11/cast.h +++ b/include/pybind11/cast.h @@ -1328,7 +1328,7 @@ struct handle_type_name { }; template <> struct handle_type_name { - static constexpr auto name = const_name("Buffer"); + static constexpr auto name = const_name("collections.abc.Buffer"); }; template <> struct handle_type_name { diff --git a/include/pybind11/stl.h b/include/pybind11/stl.h index 6a148e740..4258f1584 100644 --- a/include/pybind11/stl.h +++ b/include/pybind11/stl.h @@ -43,7 +43,7 @@ PYBIND11_NAMESPACE_BEGIN(detail) // Begin: Equivalent of // https://github.com/google/clif/blob/ae4eee1de07cdf115c0c9bf9fec9ff28efce6f6c/clif/python/runtime.cc#L388-L438 /* -The three `PyObjectTypeIsConvertibleTo*()` functions below are +The three `object_is_convertible_to_*()` functions below are the result of converging the behaviors of pybind11 and PyCLIF (http://github.com/google/clif). @@ -69,10 +69,13 @@ to prevent accidents and improve readability: are also fairly commonly used, therefore enforcing explicit conversions would have an unfavorable cost : benefit ratio; more sloppily speaking, such an enforcement would be more annoying than helpful. + +Additional checks have been added to allow types derived from `collections.abc.Set` and +`collections.abc.Mapping` (`collections.abc.Sequence` is already allowed by `PySequence_Check`). */ -inline bool PyObjectIsInstanceWithOneOfTpNames(PyObject *obj, - std::initializer_list tp_names) { +inline bool object_is_instance_with_one_of_tp_names(PyObject *obj, + std::initializer_list tp_names) { if (PyType_Check(obj)) { return false; } @@ -85,37 +88,48 @@ inline bool PyObjectIsInstanceWithOneOfTpNames(PyObject *obj, return false; } -inline bool PyObjectTypeIsConvertibleToStdVector(PyObject *obj) { - if (PySequence_Check(obj) != 0) { - return !PyUnicode_Check(obj) && !PyBytes_Check(obj); +inline bool object_is_convertible_to_std_vector(const handle &src) { + // Allow sequence-like objects, but not (byte-)string-like objects. + if (PySequence_Check(src.ptr()) != 0) { + return !PyUnicode_Check(src.ptr()) && !PyBytes_Check(src.ptr()); } - return (PyGen_Check(obj) != 0) || (PyAnySet_Check(obj) != 0) - || PyObjectIsInstanceWithOneOfTpNames( - obj, {"dict_keys", "dict_values", "dict_items", "map", "zip"}); + // Allow generators, set/frozenset and several common iterable types. + return (PyGen_Check(src.ptr()) != 0) || (PyAnySet_Check(src.ptr()) != 0) + || object_is_instance_with_one_of_tp_names( + src.ptr(), {"dict_keys", "dict_values", "dict_items", "map", "zip"}); } -inline bool PyObjectTypeIsConvertibleToStdSet(PyObject *obj) { - return (PyAnySet_Check(obj) != 0) || PyObjectIsInstanceWithOneOfTpNames(obj, {"dict_keys"}); +inline bool object_is_convertible_to_std_set(const handle &src, bool convert) { + // Allow set/frozenset and dict keys. + // In convert mode: also allow types derived from collections.abc.Set. + return ((PyAnySet_Check(src.ptr()) != 0) + || object_is_instance_with_one_of_tp_names(src.ptr(), {"dict_keys"})) + || (convert && isinstance(src, module_::import("collections.abc").attr("Set"))); } -inline bool PyObjectTypeIsConvertibleToStdMap(PyObject *obj) { - if (PyDict_Check(obj)) { +inline bool object_is_convertible_to_std_map(const handle &src, bool convert) { + // Allow dict. + if (PyDict_Check(src.ptr())) { return true; } - // Implicit requirement in the conditions below: - // A type with `.__getitem__()` & `.items()` methods must implement these - // to be compatible with https://docs.python.org/3/c-api/mapping.html - if (PyMapping_Check(obj) == 0) { - return false; + // Allow types conforming to Mapping Protocol. + // According to https://docs.python.org/3/c-api/mapping.html, `PyMappingCheck()` checks for + // `__getitem__()` without checking the type of keys. In order to restrict the allowed types + // closer to actual Mapping-like types, we also check for the `items()` method. + if (PyMapping_Check(src.ptr()) != 0) { + PyObject *items = PyObject_GetAttrString(src.ptr(), "items"); + if (items != nullptr) { + bool is_convertible = (PyCallable_Check(items) != 0); + Py_DECREF(items); + if (is_convertible) { + return true; + } + } else { + PyErr_Clear(); + } } - PyObject *items = PyObject_GetAttrString(obj, "items"); - if (items == nullptr) { - PyErr_Clear(); - return false; - } - bool is_convertible = (PyCallable_Check(items) != 0); - Py_DECREF(items); - return is_convertible; + // In convert mode: Allow types derived from collections.abc.Mapping + return convert && isinstance(src, module_::import("collections.abc").attr("Mapping")); } // @@ -172,7 +186,7 @@ private: public: bool load(handle src, bool convert) { - if (!PyObjectTypeIsConvertibleToStdSet(src.ptr())) { + if (!object_is_convertible_to_std_set(src, convert)) { return false; } if (isinstance(src)) { @@ -203,7 +217,9 @@ public: return s.release(); } - PYBIND11_TYPE_CASTER(type, const_name("set[") + key_conv::name + const_name("]")); + PYBIND11_TYPE_CASTER(type, + io_name("collections.abc.Set", "set") + const_name("[") + key_conv::name + + const_name("]")); }; template @@ -234,7 +250,7 @@ private: public: bool load(handle src, bool convert) { - if (!PyObjectTypeIsConvertibleToStdMap(src.ptr())) { + if (!object_is_convertible_to_std_map(src, convert)) { return false; } if (isinstance(src)) { @@ -274,7 +290,8 @@ public: } PYBIND11_TYPE_CASTER(Type, - const_name("dict[") + key_conv::name + const_name(", ") + value_conv::name + io_name("collections.abc.Mapping", "dict") + const_name("[") + + key_conv::name + const_name(", ") + value_conv::name + const_name("]")); }; @@ -283,7 +300,7 @@ struct list_caster { using value_conv = make_caster; bool load(handle src, bool convert) { - if (!PyObjectTypeIsConvertibleToStdVector(src.ptr())) { + if (!object_is_convertible_to_std_vector(src)) { return false; } if (isinstance(src)) { @@ -340,7 +357,9 @@ public: return l.release(); } - PYBIND11_TYPE_CASTER(Type, const_name("list[") + value_conv::name + const_name("]")); + PYBIND11_TYPE_CASTER(Type, + io_name("collections.abc.Sequence", "list") + const_name("[") + + value_conv::name + const_name("]")); }; template @@ -416,7 +435,7 @@ private: public: bool load(handle src, bool convert) { - if (!PyObjectTypeIsConvertibleToStdVector(src.ptr())) { + if (!object_is_convertible_to_std_vector(src)) { return false; } if (isinstance(src)) { @@ -474,10 +493,12 @@ public: using cast_op_type = movable_cast_op_type; static constexpr auto name - = const_name(const_name(""), const_name("Annotated[")) + const_name("list[") - + value_conv::name + const_name("]") - + const_name( - const_name(""), const_name(", FixedSize(") + const_name() + const_name(")]")); + = const_name(const_name(""), const_name("typing.Annotated[")) + + io_name("collections.abc.Sequence", "list") + const_name("[") + value_conv::name + + const_name("]") + + const_name(const_name(""), + const_name(", \"FixedSize(") + const_name() + + const_name(")\"]")); }; template diff --git a/tests/test_buffers.py b/tests/test_buffers.py index 2612edb27..d335b71e9 100644 --- a/tests/test_buffers.py +++ b/tests/test_buffers.py @@ -230,7 +230,7 @@ def test_ctypes_from_buffer(): def test_buffer_docstring(): assert ( m.get_buffer_info.__doc__.strip() - == "get_buffer_info(arg0: Buffer) -> pybind11_tests.buffers.buffer_info" + == "get_buffer_info(arg0: collections.abc.Buffer) -> pybind11_tests.buffers.buffer_info" ) diff --git a/tests/test_kwargs_and_defaults.py b/tests/test_kwargs_and_defaults.py index a8e19f15b..b62e4b741 100644 --- a/tests/test_kwargs_and_defaults.py +++ b/tests/test_kwargs_and_defaults.py @@ -22,7 +22,7 @@ def test_function_signatures(doc): assert doc(m.kw_func3) == "kw_func3(data: str = 'Hello world!') -> None" assert ( doc(m.kw_func4) - == "kw_func4(myList: list[typing.SupportsInt] = [13, 17]) -> str" + == "kw_func4(myList: collections.abc.Sequence[typing.SupportsInt] = [13, 17]) -> str" ) assert ( doc(m.kw_func_udl) diff --git a/tests/test_pytypes.py b/tests/test_pytypes.py index 697e43965..b20b09883 100644 --- a/tests/test_pytypes.py +++ b/tests/test_pytypes.py @@ -1254,7 +1254,7 @@ def test_arg_return_type_hints(doc): # std::vector assert ( doc(m.half_of_number_vector) - == "half_of_number_vector(arg0: list[Union[float, int]]) -> list[float]" + == "half_of_number_vector(arg0: collections.abc.Sequence[Union[float, int]]) -> list[float]" ) # Tuple assert ( diff --git a/tests/test_stl.cpp b/tests/test_stl.cpp index 9ddd951e0..5eff4f583 100644 --- a/tests/test_stl.cpp +++ b/tests/test_stl.cpp @@ -648,4 +648,19 @@ TEST_SUBMODULE(stl, m) { } return zum; }); + m.def("roundtrip_std_vector_int", [](const std::vector &v) { return v; }); + m.def("roundtrip_std_map_str_int", [](const std::map &m) { return m; }); + m.def("roundtrip_std_set_int", [](const std::set &s) { return s; }); + m.def( + "roundtrip_std_vector_int_noconvert", + [](const std::vector &v) { return v; }, + py::arg("v").noconvert()); + m.def( + "roundtrip_std_map_str_int_noconvert", + [](const std::map &m) { return m; }, + py::arg("m").noconvert()); + m.def( + "roundtrip_std_set_int_noconvert", + [](const std::set &s) { return s; }, + py::arg("s").noconvert()); } diff --git a/tests/test_stl.py b/tests/test_stl.py index 29a6bf119..96c84374c 100644 --- a/tests/test_stl.py +++ b/tests/test_stl.py @@ -20,7 +20,10 @@ def test_vector(doc): assert m.load_bool_vector((True, False)) assert doc(m.cast_vector) == "cast_vector() -> list[int]" - assert doc(m.load_vector) == "load_vector(arg0: list[typing.SupportsInt]) -> bool" + assert ( + doc(m.load_vector) + == "load_vector(arg0: collections.abc.Sequence[typing.SupportsInt]) -> bool" + ) # Test regression caused by 936: pointers to stl containers weren't castable assert m.cast_ptr_vector() == ["lvalue", "lvalue"] @@ -42,10 +45,13 @@ def test_array(doc): assert m.load_array(lst) assert m.load_array(tuple(lst)) - assert doc(m.cast_array) == "cast_array() -> Annotated[list[int], FixedSize(2)]" + assert ( + doc(m.cast_array) + == 'cast_array() -> typing.Annotated[list[int], "FixedSize(2)"]' + ) assert ( doc(m.load_array) - == "load_array(arg0: Annotated[list[typing.SupportsInt], FixedSize(2)]) -> bool" + == 'load_array(arg0: typing.Annotated[collections.abc.Sequence[typing.SupportsInt], "FixedSize(2)"]) -> bool' ) @@ -65,7 +71,8 @@ def test_valarray(doc): assert doc(m.cast_valarray) == "cast_valarray() -> list[int]" assert ( - doc(m.load_valarray) == "load_valarray(arg0: list[typing.SupportsInt]) -> bool" + doc(m.load_valarray) + == "load_valarray(arg0: collections.abc.Sequence[typing.SupportsInt]) -> bool" ) @@ -79,7 +86,9 @@ def test_map(doc): assert m.load_map(d) assert doc(m.cast_map) == "cast_map() -> dict[str, str]" - assert doc(m.load_map) == "load_map(arg0: dict[str, str]) -> bool" + assert ( + doc(m.load_map) == "load_map(arg0: collections.abc.Mapping[str, str]) -> bool" + ) def test_set(doc): @@ -91,7 +100,7 @@ def test_set(doc): assert m.load_set(frozenset(s)) assert doc(m.cast_set) == "cast_set() -> set[str]" - assert doc(m.load_set) == "load_set(arg0: set[str]) -> bool" + assert doc(m.load_set) == "load_set(arg0: collections.abc.Set[str]) -> bool" def test_recursive_casting(): @@ -273,7 +282,7 @@ def test_fs_path(doc): assert m.parent_paths(["foo/bar", "foo/baz"]) == [Path("foo"), Path("foo")] assert ( doc(m.parent_paths) - == "parent_paths(arg0: list[Union[os.PathLike, str, bytes]]) -> list[pathlib.Path]" + == "parent_paths(arg0: collections.abc.Sequence[Union[os.PathLike, str, bytes]]) -> list[pathlib.Path]" ) # py::typing::List assert m.parent_paths_list(["foo/bar", "foo/baz"]) == [Path("foo"), Path("foo")] @@ -364,7 +373,7 @@ def test_stl_pass_by_pointer(msg): msg(excinfo.value) == """ stl_pass_by_pointer(): incompatible function arguments. The following argument types are supported: - 1. (v: list[typing.SupportsInt] = None) -> list[int] + 1. (v: collections.abc.Sequence[typing.SupportsInt] = None) -> list[int] Invoked with: """ @@ -376,7 +385,7 @@ def test_stl_pass_by_pointer(msg): msg(excinfo.value) == """ stl_pass_by_pointer(): incompatible function arguments. The following argument types are supported: - 1. (v: list[typing.SupportsInt] = None) -> list[int] + 1. (v: collections.abc.Sequence[typing.SupportsInt] = None) -> list[int] Invoked with: None """ @@ -567,3 +576,145 @@ def test_map_caster_fully_consumes_generator_object(items, expected_exception): with pytest.raises(expected_exception): m.pass_std_map_int(FakePyMappingGenObj(gen_obj)) assert not tuple(gen_obj) + + +def test_sequence_caster_protocol(doc): + from collections.abc import Sequence + + # Implements the Sequence protocol without explicitly inheriting from collections.abc.Sequence. + class BareSequenceLike: + def __init__(self, *args): + self.data = tuple(args) + + def __len__(self): + return len(self.data) + + def __getitem__(self, index): + return self.data[index] + + # Implements the Sequence protocol by reusing BareSequenceLike's implementation. + # Additionally, inherits from collections.abc.Sequence. + class FormalSequenceLike(BareSequenceLike, Sequence): + pass + + # convert mode + assert ( + doc(m.roundtrip_std_vector_int) + == "roundtrip_std_vector_int(arg0: collections.abc.Sequence[typing.SupportsInt]) -> list[int]" + ) + assert m.roundtrip_std_vector_int([1, 2, 3]) == [1, 2, 3] + assert m.roundtrip_std_vector_int((1, 2, 3)) == [1, 2, 3] + assert m.roundtrip_std_vector_int(FormalSequenceLike(1, 2, 3)) == [1, 2, 3] + assert m.roundtrip_std_vector_int(BareSequenceLike(1, 2, 3)) == [1, 2, 3] + assert m.roundtrip_std_vector_int([]) == [] + assert m.roundtrip_std_vector_int(()) == [] + assert m.roundtrip_std_vector_int(BareSequenceLike()) == [] + # noconvert mode + assert ( + doc(m.roundtrip_std_vector_int_noconvert) + == "roundtrip_std_vector_int_noconvert(v: list[int]) -> list[int]" + ) + assert m.roundtrip_std_vector_int_noconvert([1, 2, 3]) == [1, 2, 3] + assert m.roundtrip_std_vector_int_noconvert((1, 2, 3)) == [1, 2, 3] + assert m.roundtrip_std_vector_int_noconvert(FormalSequenceLike(1, 2, 3)) == [ + 1, + 2, + 3, + ] + assert m.roundtrip_std_vector_int_noconvert(BareSequenceLike(1, 2, 3)) == [1, 2, 3] + assert m.roundtrip_std_vector_int_noconvert([]) == [] + assert m.roundtrip_std_vector_int_noconvert(()) == [] + assert m.roundtrip_std_vector_int_noconvert(BareSequenceLike()) == [] + + +def test_mapping_caster_protocol(doc): + from collections.abc import Mapping + + # Implements the Mapping protocol without explicitly inheriting from collections.abc.Mapping. + class BareMappingLike: + def __init__(self, **kwargs): + self.data = dict(kwargs) + + def __len__(self): + return len(self.data) + + def __getitem__(self, key): + return self.data[key] + + def __iter__(self): + yield from self.data + + # Implements the Mapping protocol by reusing BareMappingLike's implementation. + # Additionally, inherits from collections.abc.Mapping. + class FormalMappingLike(BareMappingLike, Mapping): + pass + + a1b2c3 = {"a": 1, "b": 2, "c": 3} + # convert mode + assert ( + doc(m.roundtrip_std_map_str_int) + == "roundtrip_std_map_str_int(arg0: collections.abc.Mapping[str, typing.SupportsInt]) -> dict[str, int]" + ) + assert m.roundtrip_std_map_str_int(a1b2c3) == a1b2c3 + assert m.roundtrip_std_map_str_int(FormalMappingLike(**a1b2c3)) == a1b2c3 + assert m.roundtrip_std_map_str_int({}) == {} + assert m.roundtrip_std_map_str_int(FormalMappingLike()) == {} + with pytest.raises(TypeError): + m.roundtrip_std_map_str_int(BareMappingLike(**a1b2c3)) + # noconvert mode + assert ( + doc(m.roundtrip_std_map_str_int_noconvert) + == "roundtrip_std_map_str_int_noconvert(m: dict[str, int]) -> dict[str, int]" + ) + assert m.roundtrip_std_map_str_int_noconvert(a1b2c3) == a1b2c3 + assert m.roundtrip_std_map_str_int_noconvert({}) == {} + with pytest.raises(TypeError): + m.roundtrip_std_map_str_int_noconvert(FormalMappingLike(**a1b2c3)) + with pytest.raises(TypeError): + m.roundtrip_std_map_str_int_noconvert(BareMappingLike(**a1b2c3)) + + +def test_set_caster_protocol(doc): + from collections.abc import Set + + # Implements the Set protocol without explicitly inheriting from collections.abc.Set. + class BareSetLike: + def __init__(self, *args): + self.data = set(args) + + def __len__(self): + return len(self.data) + + def __contains__(self, item): + return item in self.data + + def __iter__(self): + yield from self.data + + # Implements the Set protocol by reusing BareSetLike's implementation. + # Additionally, inherits from collections.abc.Set. + class FormalSetLike(BareSetLike, Set): + pass + + # convert mode + assert ( + doc(m.roundtrip_std_set_int) + == "roundtrip_std_set_int(arg0: collections.abc.Set[typing.SupportsInt]) -> set[int]" + ) + assert m.roundtrip_std_set_int({1, 2, 3}) == {1, 2, 3} + assert m.roundtrip_std_set_int(FormalSetLike(1, 2, 3)) == {1, 2, 3} + assert m.roundtrip_std_set_int(set()) == set() + assert m.roundtrip_std_set_int(FormalSetLike()) == set() + with pytest.raises(TypeError): + m.roundtrip_std_set_int(BareSetLike(1, 2, 3)) + # noconvert mode + assert ( + doc(m.roundtrip_std_set_int_noconvert) + == "roundtrip_std_set_int_noconvert(s: set[int]) -> set[int]" + ) + assert m.roundtrip_std_set_int_noconvert({1, 2, 3}) == {1, 2, 3} + assert m.roundtrip_std_set_int_noconvert(set()) == set() + with pytest.raises(TypeError): + m.roundtrip_std_set_int_noconvert(FormalSetLike(1, 2, 3)) + with pytest.raises(TypeError): + m.roundtrip_std_set_int_noconvert(BareSetLike(1, 2, 3)) From cbcc23855e0ef91ffac934f5f60555e478e16a8e Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Sun, 13 Apr 2025 19:48:32 -0700 Subject: [PATCH 2/7] Factor out pybind11/gil_simple.h (#5614) * Factor out pybind11/gil_simple.h * Remove disarm() member functions in pybind11/gil_simple.h --- CMakeLists.txt | 1 + include/pybind11/detail/internals.h | 23 ++-------- include/pybind11/gil.h | 53 ++++++++---------------- include/pybind11/gil_simple.h | 37 +++++++++++++++++ tests/extra_python_package/test_files.py | 1 + 5 files changed, 60 insertions(+), 55 deletions(-) create mode 100644 include/pybind11/gil_simple.h diff --git a/CMakeLists.txt b/CMakeLists.txt index 8fbcea524..d570112dd 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -158,6 +158,7 @@ set(PYBIND11_HEADERS include/pybind11/eval.h include/pybind11/gil.h include/pybind11/gil_safe_call_once.h + include/pybind11/gil_simple.h include/pybind11/iostream.h include/pybind11/functional.h include/pybind11/native_enum.h diff --git a/include/pybind11/detail/internals.h b/include/pybind11/detail/internals.h index 30b1fe2f9..531769585 100644 --- a/include/pybind11/detail/internals.h +++ b/include/pybind11/detail/internals.h @@ -9,15 +9,12 @@ #pragma once -#include "common.h" - -#if defined(PYBIND11_SIMPLE_GIL_MANAGEMENT) -# include -#endif - #include +#include #include +#include "common.h" + #include #include #include @@ -425,19 +422,7 @@ PYBIND11_NOINLINE internals &get_internals() { return **internals_pp; } -#if defined(PYBIND11_SIMPLE_GIL_MANAGEMENT) - gil_scoped_acquire gil; -#else - // Ensure that the GIL is held since we will need to make Python calls. - // Cannot use py::gil_scoped_acquire here since that constructor calls get_internals. - struct gil_scoped_acquire_local { - gil_scoped_acquire_local() : state(PyGILState_Ensure()) {} - gil_scoped_acquire_local(const gil_scoped_acquire_local &) = delete; - gil_scoped_acquire_local &operator=(const gil_scoped_acquire_local &) = delete; - ~gil_scoped_acquire_local() { PyGILState_Release(state); } - const PyGILState_STATE state; - } gil; -#endif + gil_scoped_acquire_simple gil; error_scope err_scope; dict state_dict = get_python_state_dict(); diff --git a/include/pybind11/gil.h b/include/pybind11/gil.h index 888810493..3f2c28f45 100644 --- a/include/pybind11/gil.h +++ b/include/pybind11/gil.h @@ -9,13 +9,24 @@ #pragma once -#include "detail/common.h" +#if defined(PYBIND11_SIMPLE_GIL_MANAGEMENT) -#include +# include "detail/common.h" +# include "gil_simple.h" -#if !defined(PYBIND11_SIMPLE_GIL_MANAGEMENT) +PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE) + +using gil_scoped_acquire = gil_scoped_acquire_simple; +using gil_scoped_release = gil_scoped_release_simple; + +PYBIND11_NAMESPACE_END(PYBIND11_NAMESPACE) + +#else + +# include "detail/common.h" # include "detail/internals.h" -#endif + +# include PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE) @@ -26,8 +37,6 @@ PyThreadState *get_thread_state_unchecked(); PYBIND11_NAMESPACE_END(detail) -#if !defined(PYBIND11_SIMPLE_GIL_MANAGEMENT) - /* The functions below essentially reproduce the PyGILState_* API using a RAII * pattern, but there are a few important differences: * @@ -182,34 +191,6 @@ private: bool active = true; }; -#else // PYBIND11_SIMPLE_GIL_MANAGEMENT - -class gil_scoped_acquire { - PyGILState_STATE state; - -public: - gil_scoped_acquire() : state{PyGILState_Ensure()} {} - gil_scoped_acquire(const gil_scoped_acquire &) = delete; - gil_scoped_acquire &operator=(const gil_scoped_acquire &) = delete; - ~gil_scoped_acquire() { PyGILState_Release(state); } - void disarm() {} -}; - -class gil_scoped_release { - PyThreadState *state; - -public: - // PRECONDITION: The GIL must be held when this constructor is called. - gil_scoped_release() { - assert(PyGILState_Check()); - state = PyEval_SaveThread(); - } - gil_scoped_release(const gil_scoped_release &) = delete; - gil_scoped_release &operator=(const gil_scoped_release &) = delete; - ~gil_scoped_release() { PyEval_RestoreThread(state); } - void disarm() {} -}; - -#endif // PYBIND11_SIMPLE_GIL_MANAGEMENT - PYBIND11_NAMESPACE_END(PYBIND11_NAMESPACE) + +#endif // !PYBIND11_SIMPLE_GIL_MANAGEMENT diff --git a/include/pybind11/gil_simple.h b/include/pybind11/gil_simple.h new file mode 100644 index 000000000..9ff428ad8 --- /dev/null +++ b/include/pybind11/gil_simple.h @@ -0,0 +1,37 @@ +// Copyright (c) 2016-2025 The Pybind Development Team. +// All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +#pragma once + +#include "detail/common.h" + +#include + +PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE) + +class gil_scoped_acquire_simple { + PyGILState_STATE state; + +public: + gil_scoped_acquire_simple() : state{PyGILState_Ensure()} {} + gil_scoped_acquire_simple(const gil_scoped_acquire_simple &) = delete; + gil_scoped_acquire_simple &operator=(const gil_scoped_acquire_simple &) = delete; + ~gil_scoped_acquire_simple() { PyGILState_Release(state); } +}; + +class gil_scoped_release_simple { + PyThreadState *state; + +public: + // PRECONDITION: The GIL must be held when this constructor is called. + gil_scoped_release_simple() { + assert(PyGILState_Check()); + state = PyEval_SaveThread(); + } + gil_scoped_release_simple(const gil_scoped_release_simple &) = delete; + gil_scoped_release_simple &operator=(const gil_scoped_release_simple &) = delete; + ~gil_scoped_release_simple() { PyEval_RestoreThread(state); } +}; + +PYBIND11_NAMESPACE_END(PYBIND11_NAMESPACE) diff --git a/tests/extra_python_package/test_files.py b/tests/extra_python_package/test_files.py index 9aca4d6d1..5c6015050 100644 --- a/tests/extra_python_package/test_files.py +++ b/tests/extra_python_package/test_files.py @@ -37,6 +37,7 @@ main_headers = { "include/pybind11/functional.h", "include/pybind11/gil.h", "include/pybind11/gil_safe_call_once.h", + "include/pybind11/gil_simple.h", "include/pybind11/iostream.h", "include/pybind11/native_enum.h", "include/pybind11/numpy.h", From b3bb31ca51c4af3fb1278f917cb9bbbe2336b812 Mon Sep 17 00:00:00 2001 From: Henry Schreiner Date: Mon, 14 Apr 2025 17:25:32 -0400 Subject: [PATCH 3/7] ci: work on speeding up further (#5613) * ci: work on speeding up further Signed-off-by: Henry Schreiner * chore: move uv's index-strategy to pyproject.toml Signed-off-by: Henry Schreiner * Update .github/workflows/ci.yml --------- Signed-off-by: Henry Schreiner --- .github/workflows/ci.yml | 44 ++++++++++-------------- .github/workflows/configure.yml | 24 ++++--------- .github/workflows/pip.yml | 24 ++++++------- pyproject.toml | 3 ++ tests/CMakeLists.txt | 7 ++-- tests/extra_python_package/test_files.py | 16 ++++++--- tests/requirements.txt | 1 + 7 files changed, 55 insertions(+), 64 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 96e5b9b93..d9308decb 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -122,33 +122,28 @@ jobs: if: runner.os == 'macOS' run: brew install boost - - name: Update CMake - uses: jwlawson/actions-setup-cmake@v2.0 - - - name: Cache wheels - if: runner.os == 'macOS' - uses: actions/cache@v4 + - name: Install uv + uses: astral-sh/setup-uv@v5 with: - # This path is specific to macOS - we really only need it for PyPy NumPy wheels - # See https://github.com/actions/cache/blob/master/examples.md#python---pip - # for ways to do this more generally - path: ~/Library/Caches/pip - # Look to see if there is a cache hit for the corresponding requirements file - key: ${{ runner.os }}-pip-${{ matrix.python }}-x64-${{ hashFiles('tests/requirements.txt') }} + enable-cache: true - name: Prepare env - run: | - python -m pip install -r tests/requirements.txt + run: uv pip install --python=python --system -r tests/requirements.txt - name: Setup annotations on Linux if: runner.os == 'Linux' - run: python -m pip install pytest-github-actions-annotate-failures + run: uv pip install --python=python --system pytest-github-actions-annotate-failures + + # TODO Resolve Windows Ninja shared object issue on Python 3.8+ + - name: Use Ninja except on Windows + if: runner.os != 'Windows' + run: echo "CMAKE_GENERATOR=Ninja" >> "$GITHUB_ENV" # First build - C++11 mode and inplace # More-or-less randomly adding -DPYBIND11_SIMPLE_GIL_MANAGEMENT=ON here - name: Configure C++11 ${{ matrix.args }} run: > - cmake -S . -B . + cmake -S. -B. -DPYBIND11_WERROR=ON -DPYBIND11_DISABLE_HANDLE_TYPE_NAME_DEFAULT_IMPLEMENTATION=ON -DPYBIND11_SIMPLE_GIL_MANAGEMENT=ON @@ -159,13 +154,13 @@ jobs: ${{ matrix.args }} - name: Build C++11 - run: cmake --build . -j 2 + run: cmake --build . - name: Python tests C++11 - run: cmake --build . --target pytest -j 2 + run: cmake --build . --target pytest - name: C++11 tests - run: cmake --build . --target cpptest -j 2 + run: cmake --build . --target cpptest - name: Interface test C++11 run: cmake --build . --target test_cmake_build @@ -177,7 +172,7 @@ jobs: # More-or-less randomly adding -DPYBIND11_SIMPLE_GIL_MANAGEMENT=OFF here. - name: Configure C++17 run: > - cmake -S . -B build2 -Werror=dev + cmake -S. -Bbuild2 -Werror=dev -DPYBIND11_WERROR=ON -DPYBIND11_SIMPLE_GIL_MANAGEMENT=OFF -DPYBIND11_PYTEST_ARGS=-v @@ -187,7 +182,7 @@ jobs: ${{ matrix.args }} - name: Build - run: cmake --build build2 -j 2 + run: cmake --build build2 - name: Python tests run: cmake --build build2 --target pytest @@ -202,7 +197,7 @@ jobs: # setuptools - name: Setuptools helpers test run: | - pip install setuptools + uv pip install --python=python --system setuptools pytest tests/extra_setuptools if: "!(matrix.runs-on == 'windows-2022')" @@ -913,10 +908,7 @@ jobs: python-version: ${{ matrix.python }} - name: Prepare env - # Ensure use of NumPy 2 (via NumPy nightlies but can be changed soon) - run: | - python3 -m pip install -r tests/requirements.txt - python3 -m pip install 'numpy>=2.0.0b1' 'scipy>=1.13.0rc1' + run: python3 -m pip install -r tests/requirements.txt - name: Update CMake uses: jwlawson/actions-setup-cmake@v2.0 diff --git a/.github/workflows/configure.yml b/.github/workflows/configure.yml index 8a5cf34d5..6408b5382 100644 --- a/.github/workflows/configure.yml +++ b/.github/workflows/configure.yml @@ -12,11 +12,6 @@ on: permissions: contents: read -env: - PIP_BREAK_SYSTEM_PACKAGES: 1 - # For cmake: - VERBOSE: 1 - jobs: # This tests various versions of CMake in various combinations, to make sure # the configure step passes. @@ -28,10 +23,10 @@ jobs: - runs-on: ubuntu-22.04 cmake: "3.15" - - runs-on: ubuntu-22.04 + - runs-on: ubuntu-24.04 cmake: "3.26" - - runs-on: ubuntu-22.04 + - runs-on: ubuntu-24.04 cmake: "3.29" - runs-on: macos-13 @@ -57,8 +52,11 @@ jobs: with: python-version: 3.11 + - name: Install uv + uses: astral-sh/setup-uv@v5 + - name: Prepare env - run: python -m pip install -r tests/requirements.txt + run: uv pip install --python=python --system -r tests/requirements.txt # An action for adding a specific version of CMake: # https://github.com/jwlawson/actions-setup-cmake @@ -68,17 +66,9 @@ jobs: cmake-version: ${{ matrix.cmake }} # These steps use a directory with a space in it intentionally - - name: Make build directories - run: mkdir "build dir" - - name: Configure - working-directory: build dir shell: bash - run: > - cmake .. - -DPYBIND11_WERROR=ON - -DDOWNLOAD_CATCH=ON - -DPYTHON_EXECUTABLE=$(python -c "import sys; print(sys.executable)") + run: cmake -S. -B"build dir" -DPYBIND11_WERROR=ON -DDOWNLOAD_CATCH=ON # Only build and test if this was manually triggered in the GitHub UI - name: Build diff --git a/.github/workflows/pip.yml b/.github/workflows/pip.yml index 79c2963b9..694b55079 100644 --- a/.github/workflows/pip.yml +++ b/.github/workflows/pip.yml @@ -15,10 +15,6 @@ on: permissions: contents: read -env: - PIP_BREAK_SYSTEM_PACKAGES: 1 - PIP_ONLY_BINARY: numpy - jobs: # This builds the sdists and wheels and makes sure the files are exactly as # expected. @@ -34,9 +30,11 @@ jobs: with: python-version: 3.8 + - name: Install uv + uses: astral-sh/setup-uv@v5 + - name: Prepare env - run: | - python -m pip install -r tests/requirements.txt + run: uv pip install --system -r tests/requirements.txt - name: Python Packaging tests run: pytest tests/extra_python_package/ @@ -56,17 +54,19 @@ jobs: with: python-version: 3.8 + - name: Install uv + uses: astral-sh/setup-uv@v5 + - name: Prepare env - run: | - python -m pip install -r tests/requirements.txt build twine + run: uv pip install --system -r tests/requirements.txt twine - name: Python Packaging tests run: pytest tests/extra_python_package/ - name: Build SDist and wheels run: | - python -m build - PYBIND11_GLOBAL_SDIST=1 python -m build + uv build + PYBIND11_GLOBAL_SDIST=1 uv build - name: Check metadata run: twine check dist/* @@ -103,7 +103,7 @@ jobs: - uses: actions/download-artifact@v4 - name: Generate artifact attestation for sdist and wheel - uses: actions/attest-build-provenance@c074443f1aee8d4aeeae555aebba3282517141b2 # v2.2.3 + uses: actions/attest-build-provenance@v2 with: subject-path: "*/pybind11*" @@ -111,10 +111,8 @@ jobs: uses: pypa/gh-action-pypi-publish@release/v1 with: packages-dir: standard/ - attestations: true - name: Publish global package uses: pypa/gh-action-pypi-publish@release/v1 with: packages-dir: global/ - attestations: true diff --git a/pyproject.toml b/pyproject.toml index 07e8edabb..38deff474 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -16,6 +16,9 @@ ignore = [ "noxfile.py", ] +# Can't use tool.uv.sources with requirements.txt +[tool.uv] +index-strategy = "unsafe-best-match" [tool.mypy] files = ["pybind11"] diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 9ee8131b1..e5e613df4 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -541,10 +541,9 @@ if(NOT CMAKE_CURRENT_SOURCE_DIR STREQUAL CMAKE_CURRENT_BINARY_DIR) endif() -# cmake 3.12 added list(transform prepend -# but we can't use it yet -string(REPLACE "test_" "${CMAKE_CURRENT_SOURCE_DIR}/test_" PYBIND11_ABS_PYTEST_FILES - "${PYBIND11_PYTEST_FILES}") +# Convert relative to full file names +list(TRANSFORM PYBIND11_PYTEST_FILES PREPEND "${CMAKE_CURRENT_SOURCE_DIR}/" + OUTPUT_VARIABLE PYBIND11_ABS_PYTEST_FILES) set(PYBIND11_TEST_PREFIX_COMMAND "" diff --git a/tests/extra_python_package/test_files.py b/tests/extra_python_package/test_files.py index 5c6015050..017b52f6a 100644 --- a/tests/extra_python_package/test_files.py +++ b/tests/extra_python_package/test_files.py @@ -2,6 +2,7 @@ from __future__ import annotations import contextlib import os +import shutil import string import subprocess import sys @@ -13,6 +14,9 @@ import zipfile DIR = os.path.abspath(os.path.dirname(__file__)) MAIN_DIR = os.path.dirname(os.path.dirname(DIR)) +HAS_UV = shutil.which("uv") is not None +UV_ARGS = ["--installer=uv"] if HAS_UV else [] + PKGCONFIG = """\ prefix=${{pcfiledir}}/../../ includedir=${{prefix}}/include @@ -168,7 +172,8 @@ def test_build_sdist(monkeypatch, tmpdir): monkeypatch.chdir(MAIN_DIR) subprocess.run( - [sys.executable, "-m", "build", "--sdist", f"--outdir={tmpdir}"], check=True + [sys.executable, "-m", "build", "--sdist", f"--outdir={tmpdir}", *UV_ARGS], + check=True, ) (sdist,) = tmpdir.visit("*.tar.gz") @@ -218,7 +223,8 @@ def test_build_global_dist(monkeypatch, tmpdir): monkeypatch.chdir(MAIN_DIR) monkeypatch.setenv("PYBIND11_GLOBAL_SDIST", "1") subprocess.run( - [sys.executable, "-m", "build", "--sdist", "--outdir", str(tmpdir)], check=True + [sys.executable, "-m", "build", "--sdist", "--outdir", str(tmpdir), *UV_ARGS], + check=True, ) (sdist,) = tmpdir.visit("*.tar.gz") @@ -266,7 +272,8 @@ def tests_build_wheel(monkeypatch, tmpdir): monkeypatch.chdir(MAIN_DIR) subprocess.run( - [sys.executable, "-m", "pip", "wheel", ".", "-w", str(tmpdir)], check=True + [sys.executable, "-m", "build", "--wheel", "--outdir", str(tmpdir), *UV_ARGS], + check=True, ) (wheel,) = tmpdir.visit("*.whl") @@ -294,7 +301,8 @@ def tests_build_global_wheel(monkeypatch, tmpdir): monkeypatch.setenv("PYBIND11_GLOBAL_SDIST", "1") subprocess.run( - [sys.executable, "-m", "pip", "wheel", ".", "-w", str(tmpdir)], check=True + [sys.executable, "-m", "build", "--wheel", "--outdir", str(tmpdir), *UV_ARGS], + check=True, ) (wheel,) = tmpdir.visit("*.whl") diff --git a/tests/requirements.txt b/tests/requirements.txt index 58c732c91..96c0cbcee 100644 --- a/tests/requirements.txt +++ b/tests/requirements.txt @@ -1,3 +1,4 @@ +--extra-index-url=https://www.graalvm.org/python/wheels --only-binary=:all: build>=1 numpy~=1.23.0; python_version=="3.8" and platform_python_implementation=="PyPy" From 3c586340fb5375cb6de34dcf2ed6de8191bd8bb0 Mon Sep 17 00:00:00 2001 From: Bryn Lloyd Date: Tue, 15 Apr 2025 08:19:06 +0200 Subject: [PATCH 4/7] Add class doc string to native_enum (#5617) * add class doc string to native_enum * adapt doc argument name * fix test, make class enum doc None by default * fix other python versions? * make clang-tidy happy * rename 'enum_doc' to 'class_doc' * update documentation * [skip ci] Polish changed documentation (mostly done by ChatGPT). --------- Co-authored-by: Bryn Lloyd <12702862+dyollb@users.noreply.github.com> Co-authored-by: Ralf W. Grosse-Kunstleve --- docs/classes.rst | 18 +++++++++++------- include/pybind11/detail/native_enum_data.h | 12 +++++++++--- include/pybind11/native_enum.h | 7 ++++--- tests/test_native_enum.cpp | 6 +++--- tests/test_native_enum.py | 6 ++++++ 5 files changed, 33 insertions(+), 16 deletions(-) diff --git a/docs/classes.rst b/docs/classes.rst index b6923963d..8788badcd 100644 --- a/docs/classes.rst +++ b/docs/classes.rst @@ -556,7 +556,7 @@ The binding code for this example looks as follows: .def_readwrite("type", &Pet::type) .def_readwrite("attr", &Pet::attr); - py::native_enum(pet, "Kind") + py::native_enum(pet, "Kind", "enum.Enum") .value("Dog", Pet::Kind::Dog) .value("Cat", Pet::Kind::Cat) .export_values() @@ -593,16 +593,20 @@ once. To achieve this, ``py::native_enum`` acts as a buffer to collect the name/value pairs. The ``.finalize()`` call uses the accumulated name/value pairs to build the arguments for constructing a native Python enum type. -The ``py::native_enum`` constructor supports a third optional -``native_type_name`` string argument, with default value ``"enum.Enum"``. -Other types can be specified like this: +The ``py::native_enum`` constructor takes a third argument, +``native_type_name``, which specifies the fully qualified name of the Python +base class to use — e.g., ``"enum.Enum"`` or ``"enum.IntEnum"``. A fourth +optional argument, ``class_doc``, provides the docstring for the generated +class. + +For example: .. code-block:: cpp - py::native_enum(pet, "Kind", "enum.IntEnum") + py::native_enum(pet, "Kind", "enum.IntEnum", "Constant specifying the kind of pet") -Any fully-qualified Python name can be specified. The only requirement is -that the named type is similar to +You may use any fully qualified Python name for ``native_type_name``. +The only requirement is that the named type is similar to `enum.Enum `_ in these ways: diff --git a/include/pybind11/detail/native_enum_data.h b/include/pybind11/detail/native_enum_data.h index 5192ed51a..b12ca6202 100644 --- a/include/pybind11/detail/native_enum_data.h +++ b/include/pybind11/detail/native_enum_data.h @@ -29,10 +29,12 @@ public: native_enum_data(const object &parent_scope, const char *enum_name, const char *native_type_name, + const char *class_doc, const std::type_index &enum_type_index) : enum_name_encoded{enum_name}, native_type_name_encoded{native_type_name}, enum_type_index{enum_type_index}, parent_scope(parent_scope), enum_name{enum_name}, - native_type_name{native_type_name}, export_values_flag{false}, finalize_needed{false} {} + native_type_name{native_type_name}, class_doc(class_doc), export_values_flag{false}, + finalize_needed{false} {} void finalize(); @@ -70,10 +72,11 @@ private: object parent_scope; str enum_name; str native_type_name; + std::string class_doc; protected: list members; - list docs; + list member_docs; bool export_values_flag : 1; // Attention: It is best to keep the bools together. private: @@ -191,7 +194,10 @@ inline void native_enum_data::finalize() { parent_scope.attr(member_name) = py_enum[member_name]; } } - for (auto doc : docs) { + if (!class_doc.empty()) { + py_enum.attr("__doc__") = class_doc.c_str(); + } + for (auto doc : member_docs) { py_enum[doc[int_(0)]].attr("__doc__") = doc[int_(1)]; } global_internals_native_enum_type_map_set_item(enum_type_index, py_enum.release().ptr()); diff --git a/include/pybind11/native_enum.h b/include/pybind11/native_enum.h index 759acd48d..5537218f2 100644 --- a/include/pybind11/native_enum.h +++ b/include/pybind11/native_enum.h @@ -24,9 +24,10 @@ public: native_enum(const object &parent_scope, const char *name, - const char *native_type_name = "enum.Enum") + const char *native_type_name, + const char *class_doc = "") : detail::native_enum_data( - parent_scope, name, native_type_name, std::type_index(typeid(EnumType))) { + parent_scope, name, native_type_name, class_doc, std::type_index(typeid(EnumType))) { if (detail::get_local_type_info(typeid(EnumType)) != nullptr || detail::get_global_type_info(typeid(EnumType)) != nullptr) { pybind11_fail( @@ -53,7 +54,7 @@ public: disarm_finalize_check("value after finalize"); members.append(make_tuple(name, static_cast(value))); if (doc) { - docs.append(make_tuple(name, doc)); + member_docs.append(make_tuple(name, doc)); } arm_finalize_check(); // There was no exception. return *this; diff --git a/tests/test_native_enum.cpp b/tests/test_native_enum.cpp index 8ed3e40ac..c8fd34df0 100644 --- a/tests/test_native_enum.cpp +++ b/tests/test_native_enum.cpp @@ -76,7 +76,7 @@ PYBIND11_NAMESPACE_END(PYBIND11_NAMESPACE) TEST_SUBMODULE(native_enum, m) { using namespace test_native_enum; - py::native_enum(m, "smallenum", "enum.IntEnum") + py::native_enum(m, "smallenum", "enum.IntEnum", "doc smallenum") .value("a", smallenum::a) .value("b", smallenum::b) .value("c", smallenum::c) @@ -89,7 +89,7 @@ TEST_SUBMODULE(native_enum, m) { .value("blue", color::blue) .finalize(); - py::native_enum(m, "altitude") + py::native_enum(m, "altitude", "enum.Enum") .value("high", altitude::high) .value("low", altitude::low) .finalize(); @@ -189,7 +189,7 @@ TEST_SUBMODULE(native_enum, m) { py::native_enum(m, "fake_double_registration_native_enum", "enum.IntEnum") .value("x", fake::x) .finalize(); - py::native_enum(m, "fake_double_registration_native_enum"); + py::native_enum(m, "fake_double_registration_native_enum", "enum.Enum"); }); m.def("native_enum_name_clash", [](py::module_ &m) { diff --git a/tests/test_native_enum.py b/tests/test_native_enum.py index f98ebd045..b942fca0d 100644 --- a/tests/test_native_enum.py +++ b/tests/test_native_enum.py @@ -108,6 +108,12 @@ def test_export_values(): assert m.exv1 is m.export_values.exv1 +def test_class_doc(): + pure_native = enum.IntEnum("pure_native", (("mem", 0),)) + assert m.smallenum.__doc__ == "doc smallenum" + assert m.color.__doc__ == pure_native.__doc__ + + def test_member_doc(): pure_native = enum.IntEnum("pure_native", (("mem", 0),)) assert m.member_doc.mem0.__doc__ == "docA" From 223e2e9da28e96142fcf883f7387cdfe2dd73f41 Mon Sep 17 00:00:00 2001 From: Bryn Lloyd Date: Wed, 16 Apr 2025 06:38:15 +0200 Subject: [PATCH 5/7] Fix missing pythonic type hints for native_enum (#5619) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix missing pythonic type hints for native_enum * Fix __qualname__ in native_enum * Rename `enum class native` → `func_sig_rendering`. Add to `ENUM_TYPES_AND_MEMBERS`. Move new code around to fit in more organically. "native" is used in so many places here, it would be very difficult to pin-point where the specific enum type is used. Similarly, "value" is difficult to pin-point. Changed to "nested_value". --------- Co-authored-by: Bryn Lloyd <12702862+dyollb@users.noreply.github.com> Co-authored-by: Ralf W. Grosse-Kunstleve --- include/pybind11/detail/native_enum_data.h | 4 +++ include/pybind11/pybind11.h | 3 ++ tests/test_native_enum.cpp | 13 +++++++++ tests/test_native_enum.py | 34 +++++++++++++++------- 4 files changed, 44 insertions(+), 10 deletions(-) diff --git a/include/pybind11/detail/native_enum_data.h b/include/pybind11/detail/native_enum_data.h index b12ca6202..26aa96598 100644 --- a/include/pybind11/detail/native_enum_data.h +++ b/include/pybind11/detail/native_enum_data.h @@ -182,6 +182,10 @@ inline void native_enum_data::finalize() { if (module_name) { py_enum.attr("__module__") = module_name; } + if (hasattr(parent_scope, "__qualname__")) { + const auto parent_qualname = parent_scope.attr("__qualname__").cast(); + py_enum.attr("__qualname__") = str(parent_qualname + "." + enum_name.cast()); + } parent_scope.attr(enum_name) = py_enum; if (export_values_flag) { for (auto member : members) { diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index 396fe7cba..c84b24d3b 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -160,6 +160,9 @@ inline std::string generate_function_signature(const char *type_caster_name_fiel handle th((PyObject *) tinfo->type); signature += th.attr("__module__").cast() + "." + th.attr("__qualname__").cast(); + } else if (auto th = detail::global_internals_native_enum_type_map_get_item(*t)) { + signature += th.attr("__module__").cast() + "." + + th.attr("__qualname__").cast(); } else if (func_rec->is_new_style_constructor && arg_index == 0) { // A new-style `__init__` takes `self` as `value_and_holder`. // Rewrite it to the proper class type. diff --git a/tests/test_native_enum.cpp b/tests/test_native_enum.cpp index c8fd34df0..c919cde2f 100644 --- a/tests/test_native_enum.cpp +++ b/tests/test_native_enum.cpp @@ -29,6 +29,8 @@ enum class member_doc { mem0, mem1, mem2 }; struct class_with_enum { enum class in_class { one, two }; + + in_class nested_value = in_class::one; }; // https://github.com/protocolbuffers/protobuf/blob/d70b5c5156858132decfdbae0a1103e6a5cb1345/src/google/protobuf/generated_enum_util.h#L52-L53 @@ -40,6 +42,8 @@ enum some_proto_enum : int { Zero, One }; template <> struct is_proto_enum : std::true_type {}; +enum class func_sig_rendering {}; + } // namespace test_native_enum PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE) @@ -124,11 +128,20 @@ TEST_SUBMODULE(native_enum, m) { .value("two", class_with_enum::in_class::two) .finalize(); + py_class_with_enum.def(py::init()) + .def_readwrite("nested_value", &class_with_enum::nested_value); + m.def("isinstance_color", [](const py::object &obj) { return py::isinstance(obj); }); m.def("pass_color", [](color e) { return static_cast(e); }); m.def("return_color", [](int i) { return static_cast(i); }); + py::native_enum(m, "func_sig_rendering", "enum.Enum").finalize(); + m.def( + "pass_and_return_func_sig_rendering", + [](func_sig_rendering e) { return e; }, + py::arg("e")); + m.def("pass_some_proto_enum", [](some_proto_enum) { return py::none(); }); m.def("return_some_proto_enum", []() { return some_proto_enum::Zero; }); diff --git a/tests/test_native_enum.py b/tests/test_native_enum.py index b942fca0d..621a9cfd4 100644 --- a/tests/test_native_enum.py +++ b/tests/test_native_enum.py @@ -54,6 +54,8 @@ MEMBER_DOC_MEMBERS = ( ("mem2", 2), ) +FUNC_SIG_RENDERING_MEMBERS = () + ENUM_TYPES_AND_MEMBERS = ( (m.smallenum, SMALLENUM_MEMBERS), (m.color, COLOR_MEMBERS), @@ -62,6 +64,7 @@ ENUM_TYPES_AND_MEMBERS = ( (m.flags_uint, FLAGS_UINT_MEMBERS), (m.export_values, EXPORT_VALUES_MEMBERS), (m.member_doc, MEMBER_DOC_MEMBERS), + (m.func_sig_rendering, FUNC_SIG_RENDERING_MEMBERS), (m.class_with_enum.in_class, CLASS_WITH_ENUM_IN_CLASS_MEMBERS), ) @@ -84,15 +87,10 @@ def test_enum_members(enum_type, members): def test_pickle_roundtrip(enum_type, members): for name, _ in members: orig = enum_type[name] - if enum_type is m.class_with_enum.in_class: - # This is a general pickle limitation. - with pytest.raises(pickle.PicklingError): - pickle.dumps(orig) - else: - # This only works if __module__ is correct. - serialized = pickle.dumps(orig) - restored = pickle.loads(serialized) - assert restored == orig + # This only works if __module__ is correct. + serialized = pickle.dumps(orig) + restored = pickle.loads(serialized) + assert restored == orig @pytest.mark.parametrize("enum_type", [m.flags_uchar, m.flags_uint]) @@ -139,7 +137,7 @@ def test_pass_color_success(): def test_pass_color_fail(): with pytest.raises(TypeError) as excinfo: m.pass_color(None) - assert "test_native_enum::color" in str(excinfo.value) + assert "pybind11_tests.native_enum.color" in str(excinfo.value) def test_return_color_success(): @@ -155,6 +153,22 @@ def test_return_color_fail(): assert str(excinfo_cast.value) == str(excinfo_direct.value) +def test_property_type_hint(): + prop = m.class_with_enum.__dict__["nested_value"] + assert isinstance(prop, property) + assert prop.fget.__doc__.startswith( + "(self: pybind11_tests.native_enum.class_with_enum)" + " -> pybind11_tests.native_enum.class_with_enum.in_class" + ) + + +def test_func_sig_rendering(): + assert m.pass_and_return_func_sig_rendering.__doc__.startswith( + "pass_and_return_func_sig_rendering(e: pybind11_tests.native_enum.func_sig_rendering)" + " -> pybind11_tests.native_enum.func_sig_rendering" + ) + + def test_type_caster_enum_type_enabled_false(): # This is really only a "does it compile" test. assert m.pass_some_proto_enum(None) is None From 5c498583cc0107778ef2d217ecccc2fb5343f253 Mon Sep 17 00:00:00 2001 From: Bryn Lloyd Date: Wed, 16 Apr 2025 14:13:23 +0200 Subject: [PATCH 6/7] fix: upgrade 20.04 runners to 22.04 (fix for ICC, NVHPC) (#5621) * Upgrade the deprecated 20.04 runners to 22.04 (ICC, NVHPC) * fix ICC compile error --------- Co-authored-by: Bryn Lloyd <12702862+dyollb@users.noreply.github.com> --- .github/workflows/ci.yml | 4 ++-- include/pybind11/chrono.h | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d9308decb..79dd6390e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -432,7 +432,7 @@ jobs: # Testing on Ubuntu + NVHPC (previous PGI) compilers, which seems to require more workarounds ubuntu-nvhpc7: - runs-on: ubuntu-20.04 + runs-on: ubuntu-22.04 name: "🐍 3 • NVHPC 23.5 • C++17 • x64" env: @@ -550,7 +550,7 @@ jobs: # Testing on ICC using the oneAPI apt repo icc: - runs-on: ubuntu-20.04 + runs-on: ubuntu-22.04 name: "🐍 3 • ICC latest • x64" diff --git a/include/pybind11/chrono.h b/include/pybind11/chrono.h index 167ea0e3d..69e59b4d6 100644 --- a/include/pybind11/chrono.h +++ b/include/pybind11/chrono.h @@ -185,7 +185,7 @@ public: using us_t = duration; auto us = duration_cast(src.time_since_epoch() % seconds(1)); if (us.count() < 0) { - us += seconds(1); + us += duration_cast(seconds(1)); } // Subtract microseconds BEFORE `system_clock::to_time_t`, because: From bc4a66dff0464d0c87291b00a3688999fea5bedc Mon Sep 17 00:00:00 2001 From: Bryn Lloyd Date: Thu, 17 Apr 2025 00:42:34 +0200 Subject: [PATCH 7/7] fix: provide useful behavior of default `py::slice` (#5620) * Change behavior of default py::slice * make clang-tidy happy * Update tests/test_pytypes.py --------- Co-authored-by: Bryn Lloyd <12702862+dyollb@users.noreply.github.com> Co-authored-by: Henry Schreiner --- include/pybind11/pytypes.h | 3 ++- tests/test_pytypes.cpp | 2 ++ tests/test_pytypes.py | 3 ++- 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h index db07a0313..32b3d2575 100644 --- a/include/pybind11/pytypes.h +++ b/include/pybind11/pytypes.h @@ -1942,13 +1942,14 @@ private: class slice : public object { public: - PYBIND11_OBJECT_DEFAULT(slice, object, PySlice_Check) + PYBIND11_OBJECT(slice, object, PySlice_Check) slice(handle start, handle stop, handle step) : object(PySlice_New(start.ptr(), stop.ptr(), step.ptr()), stolen_t{}) { if (!m_ptr) { pybind11_fail("Could not allocate slice object!"); } } + slice() : slice(none(), none(), none()) {} #ifdef PYBIND11_HAS_OPTIONAL slice(std::optional start, std::optional stop, std::optional step) diff --git a/tests/test_pytypes.cpp b/tests/test_pytypes.cpp index cbbf42178..9d7c955d8 100644 --- a/tests/test_pytypes.cpp +++ b/tests/test_pytypes.cpp @@ -671,6 +671,8 @@ TEST_SUBMODULE(pytypes, m) { m.def("test_list_slicing", [](const py::list &a) { return a[py::slice(0, -1, 2)]; }); + m.def("test_list_slicing_default", [](const py::list &a) { return a[py::slice()]; }); + // See #2361 m.def("issue2361_str_implicit_copy_none", []() { py::str is_this_none = py::none(); diff --git a/tests/test_pytypes.py b/tests/test_pytypes.py index b20b09883..d1044d995 100644 --- a/tests/test_pytypes.py +++ b/tests/test_pytypes.py @@ -603,7 +603,8 @@ def test_number_protocol(): def test_list_slicing(): li = list(range(100)) - assert li[::2] == m.test_list_slicing(li) + assert li[0:-1:2] == m.test_list_slicing(li) + assert li[::] == m.test_list_slicing_default(li) def test_issue2361():