From fe27f7faa3519f950fb974c91dfa9bf46f47a166 Mon Sep 17 00:00:00 2001 From: Maarten Baert Date: Fri, 1 Nov 2024 18:55:30 +0100 Subject: [PATCH 01/12] Add support for array_t and array_t --- include/pybind11/numpy.h | 18 +++++++ tests/test_numpy_array.cpp | 53 ++++++++++++++++++- tests/test_numpy_array.py | 105 ++++++++++++++++++++++++++----------- 3 files changed, 145 insertions(+), 31 deletions(-) diff --git a/include/pybind11/numpy.h b/include/pybind11/numpy.h index 09894cf74f..5ac6f85f0a 100644 --- a/include/pybind11/numpy.h +++ b/include/pybind11/numpy.h @@ -1436,6 +1436,24 @@ struct npy_format_descriptor +struct npy_format_descriptor> { + static constexpr auto name = const_name("object"); + + static constexpr int value = npy_api::NPY_OBJECT_; + + static pybind11::dtype dtype() { return pybind11::dtype(/*typenum*/ value); } +}; + +template <> +struct npy_format_descriptor> { + static constexpr auto name = const_name("object"); + + static constexpr int value = npy_api::NPY_OBJECT_; + + static pybind11::dtype dtype() { return pybind11::dtype(/*typenum*/ value); } +}; + #define PYBIND11_DECL_CHAR_FMT \ static constexpr auto name = const_name("S") + const_name(); \ static pybind11::dtype dtype() { \ diff --git a/tests/test_numpy_array.cpp b/tests/test_numpy_array.cpp index c2f754208b..c017f09862 100644 --- a/tests/test_numpy_array.cpp +++ b/tests/test_numpy_array.cpp @@ -528,20 +528,71 @@ TEST_SUBMODULE(numpy_array, sm) { return sum_str_values; }); + sm.def("pass_array_handle_return_sum_str_values", + [](const py::array_t &objs) { + std::string sum_str_values; + for (const auto &obj : objs) { + sum_str_values += py::str(obj.attr("value")); + } + return sum_str_values; + }); + + sm.def("pass_array_object_return_sum_str_values", + [](const py::array_t &objs) { + std::string sum_str_values; + for (const auto &obj : objs) { + sum_str_values += py::str(obj.attr("value")); + } + return sum_str_values; + }); + sm.def("pass_array_pyobject_ptr_return_as_list", [](const py::array_t &objs) -> py::list { return objs; }); + sm.def("pass_array_handle_return_as_list", + [](const py::array_t &objs) -> py::list { return objs; }); + + sm.def("pass_array_object_return_as_list", + [](const py::array_t &objs) -> py::list { return objs; }); + sm.def("return_array_pyobject_ptr_cpp_loop", [](const py::list &objs) { py::size_t arr_size = py::len(objs); py::array_t arr_from_list(static_cast(arr_size)); PyObject **data = arr_from_list.mutable_data(); for (py::size_t i = 0; i < arr_size; i++) { assert(data[i] == nullptr); - data[i] = py::cast(objs[i].attr("value")); + data[i] = py::cast(objs[i]); + } + return arr_from_list; + }); + + sm.def("return_array_handle_cpp_loop", [](const py::list &objs) { + py::size_t arr_size = py::len(objs); + py::array_t arr_from_list(static_cast(arr_size)); + py::handle *data = arr_from_list.mutable_data(); + for (py::size_t i = 0; i < arr_size; i++) { + assert(data[i] == nullptr); + data[i] = py::object(objs[i]).release(); + } + return arr_from_list; + }); + + sm.def("return_array_object_cpp_loop", [](const py::list &objs) { + py::size_t arr_size = py::len(objs); + py::array_t arr_from_list(static_cast(arr_size)); + py::object *data = arr_from_list.mutable_data(); + for (py::size_t i = 0; i < arr_size; i++) { + data[i] = objs[i]; } return arr_from_list; }); sm.def("return_array_pyobject_ptr_from_list", [](const py::list &objs) -> py::array_t { return objs; }); + + sm.def("return_array_handle_from_list", + [](const py::list &objs) -> py::array_t { return objs; }); + + sm.def("return_array_object_from_list", + [](const py::list &objs) -> py::array_t { return objs; }); } diff --git a/tests/test_numpy_array.py b/tests/test_numpy_array.py index 6e8bde826f..2bf5ad4e2c 100644 --- a/tests/test_numpy_array.py +++ b/tests/test_numpy_array.py @@ -617,8 +617,12 @@ def test_round_trip_float(): # * Sanitizers are much more likely to detect heap-use-after-free due to # other ref-count bugs. class PyValueHolder: + counter = 0 def __init__(self, value): self.value = value + PyValueHolder.counter += 1 + def __del__(self): + PyValueHolder.counter -= 1 def WrapWithPyValueHolder(*values): @@ -629,45 +633,86 @@ def UnwrapPyValueHolder(vhs): return [vh.value for vh in vhs] -def test_pass_array_pyobject_ptr_return_sum_str_values_ndarray(): - # Intentionally all temporaries, do not change. - assert ( - m.pass_array_pyobject_ptr_return_sum_str_values( - np.array(WrapWithPyValueHolder(-3, "four", 5.0), dtype=object) +@pytest.mark.parametrize( + "func", + [ + m.pass_array_pyobject_ptr_return_sum_str_values, + m.pass_array_handle_return_sum_str_values, + m.pass_array_object_return_sum_str_values, + ], +) +def test_pass_array_object_return_sum_str_values_ndarray(func): + initial_counter = PyValueHolder.counter + for loop in range(100): + # Intentionally all temporaries, do not change. + assert ( + func( + np.array(WrapWithPyValueHolder(-3, "four", 5.0), dtype=object) + ) + == "-3four5.0" ) - == "-3four5.0" - ) + assert PyValueHolder.counter == initial_counter -def test_pass_array_pyobject_ptr_return_sum_str_values_list(): - # Intentionally all temporaries, do not change. - assert ( - m.pass_array_pyobject_ptr_return_sum_str_values( - WrapWithPyValueHolder(2, "three", -4.0) +@pytest.mark.parametrize( + "func", + [ + m.pass_array_pyobject_ptr_return_sum_str_values, + m.pass_array_handle_return_sum_str_values, + m.pass_array_object_return_sum_str_values, + ], +) +def test_pass_array_object_return_sum_str_values_list(func): + initial_counter = PyValueHolder.counter + for loop in range(100): + # Intentionally all temporaries, do not change. + assert ( + func( + WrapWithPyValueHolder(2, "three", -4.0) + ) + == "2three-4.0" ) - == "2three-4.0" - ) + assert PyValueHolder.counter == initial_counter -def test_pass_array_pyobject_ptr_return_as_list(): - # Intentionally all temporaries, do not change. - assert UnwrapPyValueHolder( - m.pass_array_pyobject_ptr_return_as_list( - np.array(WrapWithPyValueHolder(-1, "two", 3.0), dtype=object) - ) - ) == [-1, "two", 3.0] +@pytest.mark.parametrize( + "func", + [ + m.pass_array_pyobject_ptr_return_as_list, + m.pass_array_handle_return_as_list, + m.pass_array_object_return_as_list, + ], +) +def test_pass_array_object_return_as_list(func): + initial_counter = PyValueHolder.counter + for loop in range(100): + # Intentionally all temporaries, do not change. + assert UnwrapPyValueHolder( + func( + np.array(WrapWithPyValueHolder(-1, "two", 3.0), dtype=object) + ) + ) == [-1, "two", 3.0] + assert PyValueHolder.counter == initial_counter @pytest.mark.parametrize( - ("return_array_pyobject_ptr", "unwrap"), + "func", [ - (m.return_array_pyobject_ptr_cpp_loop, list), - (m.return_array_pyobject_ptr_from_list, UnwrapPyValueHolder), + m.return_array_pyobject_ptr_cpp_loop, + m.return_array_handle_cpp_loop, + m.return_array_object_cpp_loop, + m.return_array_pyobject_ptr_from_list, + m.return_array_handle_from_list, + m.return_array_object_from_list, ], ) -def test_return_array_pyobject_ptr_cpp_loop(return_array_pyobject_ptr, unwrap): - # Intentionally all temporaries, do not change. - arr_from_list = return_array_pyobject_ptr(WrapWithPyValueHolder(6, "seven", -8.0)) - assert isinstance(arr_from_list, np.ndarray) - assert arr_from_list.dtype == np.dtype("O") - assert unwrap(arr_from_list) == [6, "seven", -8.0] +def test_return_array_object_cpp_loop(func): + initial_counter = PyValueHolder.counter + for loop in range(100): + # Intentionally all temporaries, do not change. + arr_from_list = func(WrapWithPyValueHolder(6, "seven", -8.0)) + assert isinstance(arr_from_list, np.ndarray) + assert arr_from_list.dtype == np.dtype("O") + assert UnwrapPyValueHolder(arr_from_list) == [6, "seven", -8.0] + del arr_from_list + assert PyValueHolder.counter == initial_counter From 67e5724996e195f3e0571500273ec61f820862dc Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Fri, 1 Nov 2024 18:09:05 +0000 Subject: [PATCH 02/12] style: pre-commit fixes --- include/pybind11/numpy.h | 4 ++-- tests/test_numpy_array.cpp | 30 ++++++++++++++---------------- tests/test_numpy_array.py | 17 +++++------------ 3 files changed, 21 insertions(+), 30 deletions(-) diff --git a/include/pybind11/numpy.h b/include/pybind11/numpy.h index 5ac6f85f0a..b7e346ef30 100644 --- a/include/pybind11/numpy.h +++ b/include/pybind11/numpy.h @@ -1437,7 +1437,7 @@ struct npy_format_descriptor -struct npy_format_descriptor> { +struct npy_format_descriptor> { static constexpr auto name = const_name("object"); static constexpr int value = npy_api::NPY_OBJECT_; @@ -1446,7 +1446,7 @@ struct npy_format_descriptor -struct npy_format_descriptor> { +struct npy_format_descriptor> { static constexpr auto name = const_name("object"); static constexpr int value = npy_api::NPY_OBJECT_; diff --git a/tests/test_numpy_array.cpp b/tests/test_numpy_array.cpp index c017f09862..a0e4273a55 100644 --- a/tests/test_numpy_array.cpp +++ b/tests/test_numpy_array.cpp @@ -528,23 +528,21 @@ TEST_SUBMODULE(numpy_array, sm) { return sum_str_values; }); - sm.def("pass_array_handle_return_sum_str_values", - [](const py::array_t &objs) { - std::string sum_str_values; - for (const auto &obj : objs) { - sum_str_values += py::str(obj.attr("value")); - } - return sum_str_values; - }); + sm.def("pass_array_handle_return_sum_str_values", [](const py::array_t &objs) { + std::string sum_str_values; + for (const auto &obj : objs) { + sum_str_values += py::str(obj.attr("value")); + } + return sum_str_values; + }); - sm.def("pass_array_object_return_sum_str_values", - [](const py::array_t &objs) { - std::string sum_str_values; - for (const auto &obj : objs) { - sum_str_values += py::str(obj.attr("value")); - } - return sum_str_values; - }); + sm.def("pass_array_object_return_sum_str_values", [](const py::array_t &objs) { + std::string sum_str_values; + for (const auto &obj : objs) { + sum_str_values += py::str(obj.attr("value")); + } + return sum_str_values; + }); sm.def("pass_array_pyobject_ptr_return_as_list", [](const py::array_t &objs) -> py::list { return objs; }); diff --git a/tests/test_numpy_array.py b/tests/test_numpy_array.py index 2bf5ad4e2c..7e7b4bec9c 100644 --- a/tests/test_numpy_array.py +++ b/tests/test_numpy_array.py @@ -618,9 +618,11 @@ def test_round_trip_float(): # other ref-count bugs. class PyValueHolder: counter = 0 + def __init__(self, value): self.value = value PyValueHolder.counter += 1 + def __del__(self): PyValueHolder.counter -= 1 @@ -646,9 +648,7 @@ def test_pass_array_object_return_sum_str_values_ndarray(func): for loop in range(100): # Intentionally all temporaries, do not change. assert ( - func( - np.array(WrapWithPyValueHolder(-3, "four", 5.0), dtype=object) - ) + func(np.array(WrapWithPyValueHolder(-3, "four", 5.0), dtype=object)) == "-3four5.0" ) assert PyValueHolder.counter == initial_counter @@ -666,12 +666,7 @@ def test_pass_array_object_return_sum_str_values_list(func): initial_counter = PyValueHolder.counter for loop in range(100): # Intentionally all temporaries, do not change. - assert ( - func( - WrapWithPyValueHolder(2, "three", -4.0) - ) - == "2three-4.0" - ) + assert func(WrapWithPyValueHolder(2, "three", -4.0)) == "2three-4.0" assert PyValueHolder.counter == initial_counter @@ -688,9 +683,7 @@ def test_pass_array_object_return_as_list(func): for loop in range(100): # Intentionally all temporaries, do not change. assert UnwrapPyValueHolder( - func( - np.array(WrapWithPyValueHolder(-1, "two", 3.0), dtype=object) - ) + func(np.array(WrapWithPyValueHolder(-1, "two", 3.0), dtype=object)) ) == [-1, "two", 3.0] assert PyValueHolder.counter == initial_counter From 02eff18444d881671f22befda2e5efee1625f370 Mon Sep 17 00:00:00 2001 From: Maarten Baert Date: Fri, 1 Nov 2024 19:22:45 +0100 Subject: [PATCH 03/12] Remove loops that aren't strictly needed --- tests/test_numpy_array.py | 36 ++++++++++++++++-------------------- 1 file changed, 16 insertions(+), 20 deletions(-) diff --git a/tests/test_numpy_array.py b/tests/test_numpy_array.py index 7e7b4bec9c..9e4d61e9d9 100644 --- a/tests/test_numpy_array.py +++ b/tests/test_numpy_array.py @@ -645,12 +645,11 @@ def UnwrapPyValueHolder(vhs): ) def test_pass_array_object_return_sum_str_values_ndarray(func): initial_counter = PyValueHolder.counter - for loop in range(100): - # Intentionally all temporaries, do not change. - assert ( - func(np.array(WrapWithPyValueHolder(-3, "four", 5.0), dtype=object)) - == "-3four5.0" - ) + # Intentionally all temporaries, do not change. + assert ( + func(np.array(WrapWithPyValueHolder(-3, "four", 5.0), dtype=object)) + == "-3four5.0" + ) assert PyValueHolder.counter == initial_counter @@ -664,9 +663,8 @@ def test_pass_array_object_return_sum_str_values_ndarray(func): ) def test_pass_array_object_return_sum_str_values_list(func): initial_counter = PyValueHolder.counter - for loop in range(100): - # Intentionally all temporaries, do not change. - assert func(WrapWithPyValueHolder(2, "three", -4.0)) == "2three-4.0" + # Intentionally all temporaries, do not change. + assert func(WrapWithPyValueHolder(2, "three", -4.0)) == "2three-4.0" assert PyValueHolder.counter == initial_counter @@ -680,11 +678,10 @@ def test_pass_array_object_return_sum_str_values_list(func): ) def test_pass_array_object_return_as_list(func): initial_counter = PyValueHolder.counter - for loop in range(100): - # Intentionally all temporaries, do not change. - assert UnwrapPyValueHolder( - func(np.array(WrapWithPyValueHolder(-1, "two", 3.0), dtype=object)) - ) == [-1, "two", 3.0] + # Intentionally all temporaries, do not change. + assert UnwrapPyValueHolder( + func(np.array(WrapWithPyValueHolder(-1, "two", 3.0), dtype=object)) + ) == [-1, "two", 3.0] assert PyValueHolder.counter == initial_counter @@ -701,11 +698,10 @@ def test_pass_array_object_return_as_list(func): ) def test_return_array_object_cpp_loop(func): initial_counter = PyValueHolder.counter - for loop in range(100): - # Intentionally all temporaries, do not change. - arr_from_list = func(WrapWithPyValueHolder(6, "seven", -8.0)) - assert isinstance(arr_from_list, np.ndarray) - assert arr_from_list.dtype == np.dtype("O") - assert UnwrapPyValueHolder(arr_from_list) == [6, "seven", -8.0] + # Intentionally all temporaries, do not change. + arr_from_list = func(WrapWithPyValueHolder(6, "seven", -8.0)) + assert isinstance(arr_from_list, np.ndarray) + assert arr_from_list.dtype == np.dtype("O") + assert UnwrapPyValueHolder(arr_from_list) == [6, "seven", -8.0] del arr_from_list assert PyValueHolder.counter == initial_counter From c573dbc946ee6143cc0e975d632e77a7d2613f3c Mon Sep 17 00:00:00 2001 From: Maarten Baert Date: Fri, 1 Nov 2024 19:32:32 +0100 Subject: [PATCH 04/12] Fix compiler warning --- tests/test_numpy_array.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_numpy_array.cpp b/tests/test_numpy_array.cpp index a0e4273a55..682aa9e2b3 100644 --- a/tests/test_numpy_array.cpp +++ b/tests/test_numpy_array.cpp @@ -569,7 +569,7 @@ TEST_SUBMODULE(numpy_array, sm) { py::array_t arr_from_list(static_cast(arr_size)); py::handle *data = arr_from_list.mutable_data(); for (py::size_t i = 0; i < arr_size; i++) { - assert(data[i] == nullptr); + assert(!data[i]); data[i] = py::object(objs[i]).release(); } return arr_from_list; From 1d49a58c9be08fabda231b869906aeef85ddad1b Mon Sep 17 00:00:00 2001 From: Maarten Baert Date: Fri, 1 Nov 2024 19:53:19 +0100 Subject: [PATCH 05/12] Disable GC-dependent checks when running on pypy or graalpy --- tests/test_numpy_array.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/tests/test_numpy_array.py b/tests/test_numpy_array.py index 9e4d61e9d9..a55fb975c4 100644 --- a/tests/test_numpy_array.py +++ b/tests/test_numpy_array.py @@ -650,7 +650,8 @@ def test_pass_array_object_return_sum_str_values_ndarray(func): func(np.array(WrapWithPyValueHolder(-3, "four", 5.0), dtype=object)) == "-3four5.0" ) - assert PyValueHolder.counter == initial_counter + if not env.PYPY and not env.GRAALPY: + assert PyValueHolder.counter == initial_counter @pytest.mark.parametrize( @@ -665,7 +666,8 @@ def test_pass_array_object_return_sum_str_values_list(func): initial_counter = PyValueHolder.counter # Intentionally all temporaries, do not change. assert func(WrapWithPyValueHolder(2, "three", -4.0)) == "2three-4.0" - assert PyValueHolder.counter == initial_counter + if not env.PYPY and not env.GRAALPY: + assert PyValueHolder.counter == initial_counter @pytest.mark.parametrize( @@ -682,7 +684,8 @@ def test_pass_array_object_return_as_list(func): assert UnwrapPyValueHolder( func(np.array(WrapWithPyValueHolder(-1, "two", 3.0), dtype=object)) ) == [-1, "two", 3.0] - assert PyValueHolder.counter == initial_counter + if not env.PYPY and not env.GRAALPY: + assert PyValueHolder.counter == initial_counter @pytest.mark.parametrize( @@ -704,4 +707,5 @@ def test_return_array_object_cpp_loop(func): assert arr_from_list.dtype == np.dtype("O") assert UnwrapPyValueHolder(arr_from_list) == [6, "seven", -8.0] del arr_from_list - assert PyValueHolder.counter == initial_counter + if not env.PYPY and not env.GRAALPY: + assert PyValueHolder.counter == initial_counter From b1fa371eae032f8d4903e67a348a56cd9e94a8a4 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Fri, 1 Nov 2024 18:54:09 +0000 Subject: [PATCH 06/12] style: pre-commit fixes --- tests/test_numpy_array.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_numpy_array.py b/tests/test_numpy_array.py index a55fb975c4..4df18b0b2d 100644 --- a/tests/test_numpy_array.py +++ b/tests/test_numpy_array.py @@ -2,7 +2,7 @@ import pytest -import env # noqa: F401 +import env from pybind11_tests import numpy_array as m np = pytest.importorskip("numpy") From 2937ac10ba1d7c5942dcefaeb776645c10fc62a7 Mon Sep 17 00:00:00 2001 From: Maarten Baert Date: Sat, 16 Nov 2024 17:27:19 +0100 Subject: [PATCH 07/12] Remove PyValueHolder counter again --- tests/test_numpy_array.py | 19 ------------------- 1 file changed, 19 deletions(-) diff --git a/tests/test_numpy_array.py b/tests/test_numpy_array.py index 4df18b0b2d..d8cf439968 100644 --- a/tests/test_numpy_array.py +++ b/tests/test_numpy_array.py @@ -617,14 +617,8 @@ def test_round_trip_float(): # * Sanitizers are much more likely to detect heap-use-after-free due to # other ref-count bugs. class PyValueHolder: - counter = 0 - def __init__(self, value): self.value = value - PyValueHolder.counter += 1 - - def __del__(self): - PyValueHolder.counter -= 1 def WrapWithPyValueHolder(*values): @@ -644,14 +638,11 @@ def UnwrapPyValueHolder(vhs): ], ) def test_pass_array_object_return_sum_str_values_ndarray(func): - initial_counter = PyValueHolder.counter # Intentionally all temporaries, do not change. assert ( func(np.array(WrapWithPyValueHolder(-3, "four", 5.0), dtype=object)) == "-3four5.0" ) - if not env.PYPY and not env.GRAALPY: - assert PyValueHolder.counter == initial_counter @pytest.mark.parametrize( @@ -663,11 +654,8 @@ def test_pass_array_object_return_sum_str_values_ndarray(func): ], ) def test_pass_array_object_return_sum_str_values_list(func): - initial_counter = PyValueHolder.counter # Intentionally all temporaries, do not change. assert func(WrapWithPyValueHolder(2, "three", -4.0)) == "2three-4.0" - if not env.PYPY and not env.GRAALPY: - assert PyValueHolder.counter == initial_counter @pytest.mark.parametrize( @@ -679,13 +667,10 @@ def test_pass_array_object_return_sum_str_values_list(func): ], ) def test_pass_array_object_return_as_list(func): - initial_counter = PyValueHolder.counter # Intentionally all temporaries, do not change. assert UnwrapPyValueHolder( func(np.array(WrapWithPyValueHolder(-1, "two", 3.0), dtype=object)) ) == [-1, "two", 3.0] - if not env.PYPY and not env.GRAALPY: - assert PyValueHolder.counter == initial_counter @pytest.mark.parametrize( @@ -700,12 +685,8 @@ def test_pass_array_object_return_as_list(func): ], ) def test_return_array_object_cpp_loop(func): - initial_counter = PyValueHolder.counter # Intentionally all temporaries, do not change. arr_from_list = func(WrapWithPyValueHolder(6, "seven", -8.0)) assert isinstance(arr_from_list, np.ndarray) assert arr_from_list.dtype == np.dtype("O") assert UnwrapPyValueHolder(arr_from_list) == [6, "seven", -8.0] - del arr_from_list - if not env.PYPY and not env.GRAALPY: - assert PyValueHolder.counter == initial_counter From 5edd99299dbb10ba4c45b270f59b442cedb633e8 Mon Sep 17 00:00:00 2001 From: Maarten Baert Date: Sat, 16 Nov 2024 18:04:13 +0100 Subject: [PATCH 08/12] Move tests to templates to avoid code duplication --- tests/test_numpy_array.cpp | 132 +++++++++++++++++-------------------- 1 file changed, 61 insertions(+), 71 deletions(-) diff --git a/tests/test_numpy_array.cpp b/tests/test_numpy_array.cpp index 682aa9e2b3..33bdf97e90 100644 --- a/tests/test_numpy_array.cpp +++ b/tests/test_numpy_array.cpp @@ -156,6 +156,55 @@ py::handle auxiliaries(T &&r, T2 &&r2) { return l.release(); } +template +PyObjectType convert_to_pyobjecttype(py::object obj); + +template <> +PyObject * convert_to_pyobjecttype(py::object obj) { + return obj.release().ptr(); +} + +template <> +py::handle convert_to_pyobjecttype(py::object obj) { + return obj.release(); +} + +template <> +py::object convert_to_pyobjecttype(py::object obj) { + return obj; +} + +template +std::string pass_array_return_sum_str_values(const py::array_t &objs) { + std::string sum_str_values; + for (const auto &obj : objs) { + sum_str_values += py::str(obj.attr("value")); + } + return sum_str_values; +} + +template +py::list pass_array_return_as_list(const py::array_t &objs) { + return objs; +} + +template +py::array_t return_array_cpp_loop(const py::list &objs) { + py::size_t arr_size = py::len(objs); + py::array_t arr_from_list(static_cast(arr_size)); + PyObjectType *data = arr_from_list.mutable_data(); + for (py::size_t i = 0; i < arr_size; i++) { + assert(!data[i]); + data[i] = convert_to_pyobjecttype(objs[i]); + } + return arr_from_list; +} + +template +py::array_t return_array_from_list(const py::list &objs) { + return objs; +} + // note: declaration at local scope would create a dangling reference! static int data_i = 42; @@ -519,78 +568,19 @@ TEST_SUBMODULE(numpy_array, sm) { sm.def("round_trip_float", [](double d) { return d; }); - sm.def("pass_array_pyobject_ptr_return_sum_str_values", - [](const py::array_t &objs) { - std::string sum_str_values; - for (const auto &obj : objs) { - sum_str_values += py::str(obj.attr("value")); - } - return sum_str_values; - }); - - sm.def("pass_array_handle_return_sum_str_values", [](const py::array_t &objs) { - std::string sum_str_values; - for (const auto &obj : objs) { - sum_str_values += py::str(obj.attr("value")); - } - return sum_str_values; - }); - - sm.def("pass_array_object_return_sum_str_values", [](const py::array_t &objs) { - std::string sum_str_values; - for (const auto &obj : objs) { - sum_str_values += py::str(obj.attr("value")); - } - return sum_str_values; - }); - - sm.def("pass_array_pyobject_ptr_return_as_list", - [](const py::array_t &objs) -> py::list { return objs; }); - - sm.def("pass_array_handle_return_as_list", - [](const py::array_t &objs) -> py::list { return objs; }); - - sm.def("pass_array_object_return_as_list", - [](const py::array_t &objs) -> py::list { return objs; }); - - sm.def("return_array_pyobject_ptr_cpp_loop", [](const py::list &objs) { - py::size_t arr_size = py::len(objs); - py::array_t arr_from_list(static_cast(arr_size)); - PyObject **data = arr_from_list.mutable_data(); - for (py::size_t i = 0; i < arr_size; i++) { - assert(data[i] == nullptr); - data[i] = py::cast(objs[i]); - } - return arr_from_list; - }); - - sm.def("return_array_handle_cpp_loop", [](const py::list &objs) { - py::size_t arr_size = py::len(objs); - py::array_t arr_from_list(static_cast(arr_size)); - py::handle *data = arr_from_list.mutable_data(); - for (py::size_t i = 0; i < arr_size; i++) { - assert(!data[i]); - data[i] = py::object(objs[i]).release(); - } - return arr_from_list; - }); - - sm.def("return_array_object_cpp_loop", [](const py::list &objs) { - py::size_t arr_size = py::len(objs); - py::array_t arr_from_list(static_cast(arr_size)); - py::object *data = arr_from_list.mutable_data(); - for (py::size_t i = 0; i < arr_size; i++) { - data[i] = objs[i]; - } - return arr_from_list; - }); + sm.def("pass_array_pyobject_ptr_return_sum_str_values", pass_array_return_sum_str_values); + sm.def("pass_array_handle_return_sum_str_values", pass_array_return_sum_str_values); + sm.def("pass_array_object_return_sum_str_values", pass_array_return_sum_str_values); - sm.def("return_array_pyobject_ptr_from_list", - [](const py::list &objs) -> py::array_t { return objs; }); + sm.def("pass_array_pyobject_ptr_return_as_list", pass_array_return_as_list); + sm.def("pass_array_handle_return_as_list", pass_array_return_as_list); + sm.def("pass_array_object_return_as_list", pass_array_return_as_list); - sm.def("return_array_handle_from_list", - [](const py::list &objs) -> py::array_t { return objs; }); + sm.def("return_array_pyobject_ptr_cpp_loop", return_array_cpp_loop); + sm.def("return_array_handle_cpp_loop", return_array_cpp_loop); + sm.def("return_array_object_cpp_loop", return_array_cpp_loop); - sm.def("return_array_object_from_list", - [](const py::list &objs) -> py::array_t { return objs; }); + sm.def("return_array_pyobject_ptr_from_list", return_array_from_list); + sm.def("return_array_handle_from_list", return_array_from_list); + sm.def("return_array_object_from_list", return_array_from_list); } From fbcc8e142f51bda0a53a70436795a30f1216facb Mon Sep 17 00:00:00 2001 From: Maarten Baert Date: Sat, 16 Nov 2024 18:06:26 +0100 Subject: [PATCH 09/12] Rerun pre-commit --- tests/test_numpy_array.cpp | 11 +++++++---- tests/test_numpy_array.py | 1 - 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/tests/test_numpy_array.cpp b/tests/test_numpy_array.cpp index 33bdf97e90..b484d58742 100644 --- a/tests/test_numpy_array.cpp +++ b/tests/test_numpy_array.cpp @@ -160,7 +160,7 @@ template PyObjectType convert_to_pyobjecttype(py::object obj); template <> -PyObject * convert_to_pyobjecttype(py::object obj) { +PyObject *convert_to_pyobjecttype(py::object obj) { return obj.release().ptr(); } @@ -568,9 +568,12 @@ TEST_SUBMODULE(numpy_array, sm) { sm.def("round_trip_float", [](double d) { return d; }); - sm.def("pass_array_pyobject_ptr_return_sum_str_values", pass_array_return_sum_str_values); - sm.def("pass_array_handle_return_sum_str_values", pass_array_return_sum_str_values); - sm.def("pass_array_object_return_sum_str_values", pass_array_return_sum_str_values); + sm.def("pass_array_pyobject_ptr_return_sum_str_values", + pass_array_return_sum_str_values); + sm.def("pass_array_handle_return_sum_str_values", + pass_array_return_sum_str_values); + sm.def("pass_array_object_return_sum_str_values", + pass_array_return_sum_str_values); sm.def("pass_array_pyobject_ptr_return_as_list", pass_array_return_as_list); sm.def("pass_array_handle_return_as_list", pass_array_return_as_list); diff --git a/tests/test_numpy_array.py b/tests/test_numpy_array.py index d8cf439968..40075ed6e5 100644 --- a/tests/test_numpy_array.py +++ b/tests/test_numpy_array.py @@ -2,7 +2,6 @@ import pytest -import env from pybind11_tests import numpy_array as m np = pytest.importorskip("numpy") From 1e2867520414d406131612676eb22272d351bf5f Mon Sep 17 00:00:00 2001 From: Maarten Baert Date: Sat, 16 Nov 2024 18:15:10 +0100 Subject: [PATCH 10/12] Restore import that was erroneously removed by pre-commit --- tests/test_numpy_array.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/test_numpy_array.py b/tests/test_numpy_array.py index 40075ed6e5..f731017def 100644 --- a/tests/test_numpy_array.py +++ b/tests/test_numpy_array.py @@ -2,6 +2,7 @@ import pytest +import env # noqa: F401 from pybind11_tests import numpy_array as m np = pytest.importorskip("numpy") From cfe6521e47fd218876922c74591c32e142d7827c Mon Sep 17 00:00:00 2001 From: Maarten Baert Date: Sat, 16 Nov 2024 18:21:56 +0100 Subject: [PATCH 11/12] Reduce code duplication with more template magic --- include/pybind11/numpy.h | 24 +++++------------------- 1 file changed, 5 insertions(+), 19 deletions(-) diff --git a/include/pybind11/numpy.h b/include/pybind11/numpy.h index b7e346ef30..228e02c3d8 100644 --- a/include/pybind11/numpy.h +++ b/include/pybind11/numpy.h @@ -1428,25 +1428,11 @@ struct npy_format_descriptor< }; template -struct npy_format_descriptor::value>> { - static constexpr auto name = const_name("object"); - - static constexpr int value = npy_api::NPY_OBJECT_; - - static pybind11::dtype dtype() { return pybind11::dtype(/*typenum*/ value); } -}; - -template <> -struct npy_format_descriptor> { - static constexpr auto name = const_name("object"); - - static constexpr int value = npy_api::NPY_OBJECT_; - - static pybind11::dtype dtype() { return pybind11::dtype(/*typenum*/ value); } -}; - -template <> -struct npy_format_descriptor> { +struct npy_format_descriptor< + T, + enable_if_t::value + || ((std::is_same::value || std::is_same::value) + && sizeof(T) == sizeof(PyObject *))>> { static constexpr auto name = const_name("object"); static constexpr int value = npy_api::NPY_OBJECT_; From a1b7094a5507ef243dad2deea1cb44bef993bc0d Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Sat, 16 Nov 2024 12:33:55 -0800 Subject: [PATCH 12/12] Bring back `.attr("value")` in `return_array_cpp_loop()` This was meant to further stress-test correctness of refcount handling. All modified test functions were manually leak-checked (`while True`, top command, Python 3.12.3, Ubuntu 24.01, gcc 13.2.0). --- tests/test_numpy_array.cpp | 2 +- tests/test_numpy_array.py | 55 ++++++++++++++++++-------------------- 2 files changed, 27 insertions(+), 30 deletions(-) diff --git a/tests/test_numpy_array.cpp b/tests/test_numpy_array.cpp index b484d58742..79ade3ba1a 100644 --- a/tests/test_numpy_array.cpp +++ b/tests/test_numpy_array.cpp @@ -195,7 +195,7 @@ py::array_t return_array_cpp_loop(const py::list &objs) { PyObjectType *data = arr_from_list.mutable_data(); for (py::size_t i = 0; i < arr_size; i++) { assert(!data[i]); - data[i] = convert_to_pyobjecttype(objs[i]); + data[i] = convert_to_pyobjecttype(objs[i].attr("value")); } return arr_from_list; } diff --git a/tests/test_numpy_array.py b/tests/test_numpy_array.py index f731017def..b1c6875f9e 100644 --- a/tests/test_numpy_array.py +++ b/tests/test_numpy_array.py @@ -629,64 +629,61 @@ def UnwrapPyValueHolder(vhs): return [vh.value for vh in vhs] +PASS_ARRAY_PYOBJECT_RETURN_SUM_STR_VALUES_FUNCTIONS = [ + m.pass_array_pyobject_ptr_return_sum_str_values, + m.pass_array_handle_return_sum_str_values, + m.pass_array_object_return_sum_str_values, +] + + @pytest.mark.parametrize( - "func", - [ - m.pass_array_pyobject_ptr_return_sum_str_values, - m.pass_array_handle_return_sum_str_values, - m.pass_array_object_return_sum_str_values, - ], + "pass_array", PASS_ARRAY_PYOBJECT_RETURN_SUM_STR_VALUES_FUNCTIONS ) -def test_pass_array_object_return_sum_str_values_ndarray(func): +def test_pass_array_object_return_sum_str_values_ndarray(pass_array): # Intentionally all temporaries, do not change. assert ( - func(np.array(WrapWithPyValueHolder(-3, "four", 5.0), dtype=object)) + pass_array(np.array(WrapWithPyValueHolder(-3, "four", 5.0), dtype=object)) == "-3four5.0" ) @pytest.mark.parametrize( - "func", - [ - m.pass_array_pyobject_ptr_return_sum_str_values, - m.pass_array_handle_return_sum_str_values, - m.pass_array_object_return_sum_str_values, - ], + "pass_array", PASS_ARRAY_PYOBJECT_RETURN_SUM_STR_VALUES_FUNCTIONS ) -def test_pass_array_object_return_sum_str_values_list(func): +def test_pass_array_object_return_sum_str_values_list(pass_array): # Intentionally all temporaries, do not change. - assert func(WrapWithPyValueHolder(2, "three", -4.0)) == "2three-4.0" + assert pass_array(WrapWithPyValueHolder(2, "three", -4.0)) == "2three-4.0" @pytest.mark.parametrize( - "func", + "pass_array", [ m.pass_array_pyobject_ptr_return_as_list, m.pass_array_handle_return_as_list, m.pass_array_object_return_as_list, ], ) -def test_pass_array_object_return_as_list(func): +def test_pass_array_object_return_as_list(pass_array): # Intentionally all temporaries, do not change. assert UnwrapPyValueHolder( - func(np.array(WrapWithPyValueHolder(-1, "two", 3.0), dtype=object)) + pass_array(np.array(WrapWithPyValueHolder(-1, "two", 3.0), dtype=object)) ) == [-1, "two", 3.0] @pytest.mark.parametrize( - "func", + ("return_array", "unwrap"), [ - m.return_array_pyobject_ptr_cpp_loop, - m.return_array_handle_cpp_loop, - m.return_array_object_cpp_loop, - m.return_array_pyobject_ptr_from_list, - m.return_array_handle_from_list, - m.return_array_object_from_list, + (m.return_array_pyobject_ptr_cpp_loop, list), + (m.return_array_handle_cpp_loop, list), + (m.return_array_object_cpp_loop, list), + (m.return_array_pyobject_ptr_from_list, UnwrapPyValueHolder), + (m.return_array_handle_from_list, UnwrapPyValueHolder), + (m.return_array_object_from_list, UnwrapPyValueHolder), ], ) -def test_return_array_object_cpp_loop(func): +def test_return_array_object_cpp_loop(return_array, unwrap): # Intentionally all temporaries, do not change. - arr_from_list = func(WrapWithPyValueHolder(6, "seven", -8.0)) + arr_from_list = return_array(WrapWithPyValueHolder(6, "seven", -8.0)) assert isinstance(arr_from_list, np.ndarray) assert arr_from_list.dtype == np.dtype("O") - assert UnwrapPyValueHolder(arr_from_list) == [6, "seven", -8.0] + assert unwrap(arr_from_list) == [6, "seven", -8.0]