From 34893a43521bb442c57982a54faf3c56f5921f21 Mon Sep 17 00:00:00 2001 From: Yannick Jadoul Date: Thu, 26 Nov 2020 23:13:39 +0100 Subject: [PATCH 01/11] Only allow integer type_caster to call __int__ or __index__ method when conversion is allowed --- include/pybind11/cast.h | 2 ++ tests/test_builtin_casters.cpp | 4 ++++ tests/test_builtin_casters.py | 35 ++++++++++++++++++++++++++++++++++ 3 files changed, 41 insertions(+) diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h index c428e3f13a..4f1628195b 100644 --- a/include/pybind11/cast.h +++ b/include/pybind11/cast.h @@ -1027,6 +1027,8 @@ struct type_caster::value && !is_std_char_t return false; } else if (PyFloat_Check(src.ptr())) { return false; + } else if (!convert && !PyLong_Check(src.ptr())) { + return false; } else if (std::is_unsigned::value) { py_value = as_unsigned(src.ptr()); } else { // signed integer: diff --git a/tests/test_builtin_casters.cpp b/tests/test_builtin_casters.cpp index acc9f8fb36..6fbabde584 100644 --- a/tests/test_builtin_casters.cpp +++ b/tests/test_builtin_casters.cpp @@ -98,6 +98,10 @@ TEST_SUBMODULE(builtin_casters, m) { m.def("i64_str", [](std::int64_t v) { return std::to_string(v); }); m.def("u64_str", [](std::uint64_t v) { return std::to_string(v); }); + // test_int_convert + m.def("int_passthrough", [](int arg) { return arg; }); + m.def("int_passthrough_noconvert", [](int arg) { return arg; }, py::arg().noconvert()); + // test_tuple m.def("pair_passthrough", [](std::pair input) { return std::make_pair(input.second, input.first); diff --git a/tests/test_builtin_casters.py b/tests/test_builtin_casters.py index bd7996b620..77b8ad8617 100644 --- a/tests/test_builtin_casters.py +++ b/tests/test_builtin_casters.py @@ -251,6 +251,41 @@ def test_integer_casting(): assert "incompatible function arguments" in str(excinfo.value) +def test_int_convert(): + class DeepThought: + def __int__(self): + return 42 + + class ShallowThought: + pass + + class AlternativeThought: + def __index__(self): + return 54 + + class FuzzyThought: + def __float__(self): + return 41.99999 + + convert, noconvert = m.int_passthrough, m.int_passthrough_noconvert + + def require_implicit(v): + pytest.raises(TypeError, noconvert, v) + + def cant_convert(v): + pytest.raises(TypeError, convert, v) + + assert convert(7) == 7 + assert noconvert(7) == 7 + cant_convert(3.14159) + assert convert(DeepThought()) == 42 + require_implicit(DeepThought()) + cant_convert(ShallowThought()) + assert convert(AlternativeThought()) == 54 + require_implicit(AlternativeThought()) + cant_convert(FuzzyThought()) + + def test_tuple(doc): """std::pair <-> tuple & std::tuple <-> tuple""" assert m.pair_passthrough((True, "test")) == ("test", True) From dc28416c5fe848f899e44b3b8705a38b0859ebd2 Mon Sep 17 00:00:00 2001 From: Yannick Jadoul Date: Fri, 27 Nov 2020 00:02:48 +0100 Subject: [PATCH 02/11] Remove tests for __index__ as this seems to only be used to convert to int in 3.8+ --- tests/test_builtin_casters.py | 6 ------ 1 file changed, 6 deletions(-) diff --git a/tests/test_builtin_casters.py b/tests/test_builtin_casters.py index 77b8ad8617..bbe59ff7e3 100644 --- a/tests/test_builtin_casters.py +++ b/tests/test_builtin_casters.py @@ -259,10 +259,6 @@ def __int__(self): class ShallowThought: pass - class AlternativeThought: - def __index__(self): - return 54 - class FuzzyThought: def __float__(self): return 41.99999 @@ -281,8 +277,6 @@ def cant_convert(v): assert convert(DeepThought()) == 42 require_implicit(DeepThought()) cant_convert(ShallowThought()) - assert convert(AlternativeThought()) == 54 - require_implicit(AlternativeThought()) cant_convert(FuzzyThought()) From 8f00086e018066f6a1b003e2e88c4d78ea9ae740 Mon Sep 17 00:00:00 2001 From: Yannick Jadoul Date: Fri, 27 Nov 2020 00:09:50 +0100 Subject: [PATCH 03/11] Take both `int` and `long` types into account for Python 2 --- include/pybind11/cast.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h index 4f1628195b..ff8234374c 100644 --- a/include/pybind11/cast.h +++ b/include/pybind11/cast.h @@ -1027,7 +1027,7 @@ struct type_caster::value && !is_std_char_t return false; } else if (PyFloat_Check(src.ptr())) { return false; - } else if (!convert && !PyLong_Check(src.ptr())) { + } else if (!convert && !PYBIND11_LONG_CHECK(src.ptr())) { return false; } else if (std::is_unsigned::value) { py_value = as_unsigned(src.ptr()); From 05d1a03b2b213b32478bda89f2f07465c85f5d96 Mon Sep 17 00:00:00 2001 From: Yannick Jadoul Date: Tue, 29 Dec 2020 15:04:59 +0100 Subject: [PATCH 04/11] Add test_numpy_int_convert to assert tests currently fail, even though np.intc has an __index__ method --- tests/test_builtin_casters.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/tests/test_builtin_casters.py b/tests/test_builtin_casters.py index bbe59ff7e3..21074ac1af 100644 --- a/tests/test_builtin_casters.py +++ b/tests/test_builtin_casters.py @@ -280,6 +280,22 @@ def cant_convert(v): cant_convert(FuzzyThought()) +def test_numpy_int_convert(): + np = pytest.importorskip("numpy") + + convert, noconvert = m.int_passthrough, m.int_passthrough_noconvert + + def require_implicit(v): + pytest.raises(TypeError, noconvert, v) + + # `np.intc` is an alias that corresponds to a C++ `int` + assert convert(np.intc(42)) == 42 + assert noconvert(np.intc(42)) == 42 + + assert convert(np.float32(3.14159)) == 3 # This might be wrong/unwanted? + require_implicit(np.float32(3.14159)) + + def test_tuple(doc): """std::pair <-> tuple & std::tuple <-> tuple""" assert m.pair_passthrough((True, "test")) == ("test", True) From cecbb98fa512ca36c2fc60ef90980e20e3aaa0a3 Mon Sep 17 00:00:00 2001 From: Yannick Jadoul Date: Tue, 29 Dec 2020 15:27:53 +0100 Subject: [PATCH 05/11] Also consider __index__ as noconvert to a C++ integer --- include/pybind11/cast.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h index ff8234374c..973a4e7c82 100644 --- a/include/pybind11/cast.h +++ b/include/pybind11/cast.h @@ -1027,7 +1027,7 @@ struct type_caster::value && !is_std_char_t return false; } else if (PyFloat_Check(src.ptr())) { return false; - } else if (!convert && !PYBIND11_LONG_CHECK(src.ptr())) { + } else if (!convert && !PYBIND11_LONG_CHECK(src.ptr()) && !PyIndex_Check(src.ptr())) { return false; } else if (std::is_unsigned::value) { py_value = as_unsigned(src.ptr()); From c4f25f84dbebf9eeddb4fa9b89661e3141313a87 Mon Sep 17 00:00:00 2001 From: Yannick Jadoul Date: Tue, 29 Dec 2020 16:15:55 +0100 Subject: [PATCH 06/11] New-style classes for Python 2.7; sigh --- tests/test_builtin_casters.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/test_builtin_casters.py b/tests/test_builtin_casters.py index 21074ac1af..053148e9a9 100644 --- a/tests/test_builtin_casters.py +++ b/tests/test_builtin_casters.py @@ -252,14 +252,14 @@ def test_integer_casting(): def test_int_convert(): - class DeepThought: + class DeepThought(object): def __int__(self): return 42 - class ShallowThought: + class ShallowThought(object): pass - class FuzzyThought: + class FuzzyThought(object): def __float__(self): return 41.99999 From 1380611d6631235e21b2d48bac4062d7581360c7 Mon Sep 17 00:00:00 2001 From: Yannick Jadoul Date: Tue, 29 Dec 2020 16:32:17 +0100 Subject: [PATCH 07/11] Add some tests on types with custom __index__ method --- tests/test_builtin_casters.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/tests/test_builtin_casters.py b/tests/test_builtin_casters.py index 053148e9a9..496412796d 100644 --- a/tests/test_builtin_casters.py +++ b/tests/test_builtin_casters.py @@ -263,6 +263,17 @@ class FuzzyThought(object): def __float__(self): return 41.99999 + class IndexedThought(object): + def __index__(self): + return 42 + + class RaisingThought(object): + def __index__(self): + raise ValueError + + def __int__(self): + return 42 + convert, noconvert = m.int_passthrough, m.int_passthrough_noconvert def require_implicit(v): @@ -278,6 +289,11 @@ def cant_convert(v): require_implicit(DeepThought()) cant_convert(ShallowThought()) cant_convert(FuzzyThought()) + if not env.PY2: + # I have no clue why __index__ is not picked up by Python 2's PyIndex_check + assert convert(IndexedThought()) == 42 + assert noconvert(IndexedThought()) == 42 + cant_convert(RaisingThought()) # no fall-back to `__int__`if `__index__` raises def test_numpy_int_convert(): From 915423de979a44a27be4f52c5378733a6c40e26f Mon Sep 17 00:00:00 2001 From: Yannick Jadoul Date: Tue, 29 Dec 2020 20:34:24 +0100 Subject: [PATCH 08/11] Ignore some tests in Python <3.8 --- include/pybind11/cast.h | 2 +- tests/test_builtin_casters.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h index 973a4e7c82..815d449916 100644 --- a/include/pybind11/cast.h +++ b/include/pybind11/cast.h @@ -1027,7 +1027,7 @@ struct type_caster::value && !is_std_char_t return false; } else if (PyFloat_Check(src.ptr())) { return false; - } else if (!convert && !PYBIND11_LONG_CHECK(src.ptr()) && !PyIndex_Check(src.ptr())) { + } else if (!convert && !PyIndex_Check(src.ptr()) && !PYBIND11_LONG_CHECK(src.ptr())) { return false; } else if (std::is_unsigned::value) { py_value = as_unsigned(src.ptr()); diff --git a/tests/test_builtin_casters.py b/tests/test_builtin_casters.py index 496412796d..bbea8227c8 100644 --- a/tests/test_builtin_casters.py +++ b/tests/test_builtin_casters.py @@ -289,8 +289,8 @@ def cant_convert(v): require_implicit(DeepThought()) cant_convert(ShallowThought()) cant_convert(FuzzyThought()) - if not env.PY2: - # I have no clue why __index__ is not picked up by Python 2's PyIndex_check + if env.PY >= (3, 8): + # Before Python 3.8, `int(obj)` does not pick up on `obj.__index__` assert convert(IndexedThought()) == 42 assert noconvert(IndexedThought()) == 42 cant_convert(RaisingThought()) # no fall-back to `__int__`if `__index__` raises From 3609fe51dbdf00ce5a3dd56f07db60078a3a10bc Mon Sep 17 00:00:00 2001 From: Yannick Jadoul Date: Wed, 30 Dec 2020 13:46:06 +0100 Subject: [PATCH 09/11] Update comment about conversion from np.float32 to C++ int --- tests/test_builtin_casters.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/test_builtin_casters.py b/tests/test_builtin_casters.py index bbea8227c8..1d48a2f596 100644 --- a/tests/test_builtin_casters.py +++ b/tests/test_builtin_casters.py @@ -308,7 +308,8 @@ def require_implicit(v): assert convert(np.intc(42)) == 42 assert noconvert(np.intc(42)) == 42 - assert convert(np.float32(3.14159)) == 3 # This might be wrong/unwanted? + # The implicit conversion from np.float32 is undesirable but currently accepted. + assert convert(np.float32(3.14159)) == 3 require_implicit(np.float32(3.14159)) From 0e89b14e92324c0257dd2a82d101c77efe3c09d9 Mon Sep 17 00:00:00 2001 From: Yannick Jadoul Date: Sun, 17 Jan 2021 01:26:20 +0100 Subject: [PATCH 10/11] Workaround difference between CPython and PyPy's different PyIndex_Check (unnoticed because we currently don't have PyPy >= 3.8) --- include/pybind11/cast.h | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h index 815d449916..f6448c9ce7 100644 --- a/include/pybind11/cast.h +++ b/include/pybind11/cast.h @@ -1020,6 +1020,14 @@ struct type_caster::value && !is_std_char_t if (!src) return false; +#if !defined(PYPY_VERSION) + auto index_check = [](PyObject *o) { return PyIndex_Check(o); }; +#else + // In PyPy 7.3.3, `PyIndex_Check` is implemented by calling `__index__`, + // while CPython only considers the existence of `nb_index`/`__index__`. + auto index_check = [](PyObject *o) { return hasattr(o, "__index__"); }; +#endif + if (std::is_floating_point::value) { if (convert || PyFloat_Check(src.ptr())) py_value = (py_type) PyFloat_AsDouble(src.ptr()); @@ -1027,7 +1035,7 @@ struct type_caster::value && !is_std_char_t return false; } else if (PyFloat_Check(src.ptr())) { return false; - } else if (!convert && !PyIndex_Check(src.ptr()) && !PYBIND11_LONG_CHECK(src.ptr())) { + } else if (!convert && !index_check(src.ptr()) && !PYBIND11_LONG_CHECK(src.ptr())) { return false; } else if (std::is_unsigned::value) { py_value = as_unsigned(src.ptr()); From b942b62733c5a1e893030376fb2967a0f872fd77 Mon Sep 17 00:00:00 2001 From: Yannick Jadoul Date: Sun, 17 Jan 2021 01:55:12 +0100 Subject: [PATCH 11/11] Avoid ICC segfault with py::arg() --- tests/test_builtin_casters.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_builtin_casters.cpp b/tests/test_builtin_casters.cpp index 6fbabde584..84c06a889b 100644 --- a/tests/test_builtin_casters.cpp +++ b/tests/test_builtin_casters.cpp @@ -100,7 +100,7 @@ TEST_SUBMODULE(builtin_casters, m) { // test_int_convert m.def("int_passthrough", [](int arg) { return arg; }); - m.def("int_passthrough_noconvert", [](int arg) { return arg; }, py::arg().noconvert()); + m.def("int_passthrough_noconvert", [](int arg) { return arg; }, py::arg{}.noconvert()); // test_tuple m.def("pair_passthrough", [](std::pair input) {