From 3a74bc41ef90c56bb3df9104ade7623c83489646 Mon Sep 17 00:00:00 2001 From: "woody.chow" Date: Thu, 6 Sep 2018 11:06:29 +0900 Subject: [PATCH 01/11] Support arbitrary type in item accessor's subscript operator --- include/pybind11/pytypes.h | 20 +++++--------------- tests/test_pytypes.cpp | 4 ++++ tests/test_pytypes.py | 1 + 3 files changed, 10 insertions(+), 15 deletions(-) diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h index f9625e77e..f7a8600e3 100644 --- a/include/pybind11/pytypes.h +++ b/include/pybind11/pytypes.h @@ -92,11 +92,8 @@ public: subclass causes a corresponding call to ``__getitem__``. Assigning a `handle` or `object` subclass causes a call to ``__setitem__``. \endrst */ - item_accessor operator[](handle key) const; - /// See above (the only difference is that the key's reference is stolen) - item_accessor operator[](object &&key) const; - /// See above (the only difference is that the key is provided as a string literal) - item_accessor operator[](const char *key) const; + template + item_accessor operator[](T &&key) const; /** \rst Return an internal functor to access the object's attributes. Casting the @@ -2270,16 +2267,9 @@ iterator object_api::end() const { return iterator::sentinel(); } template -item_accessor object_api::operator[](handle key) const { - return {derived(), reinterpret_borrow(key)}; -} -template -item_accessor object_api::operator[](object &&key) const { - return {derived(), std::move(key)}; -} -template -item_accessor object_api::operator[](const char *key) const { - return {derived(), pybind11::str(key)}; +template +item_accessor object_api::operator[](T &&key) const { + return {derived(), reinterpret_borrow(detail::object_or_cast(std::forward(key)))}; } template obj_attr_accessor object_api::attr(handle key) const { diff --git a/tests/test_pytypes.cpp b/tests/test_pytypes.cpp index cb81007c3..ef937db2f 100644 --- a/tests/test_pytypes.cpp +++ b/tests/test_pytypes.cpp @@ -126,6 +126,10 @@ TEST_SUBMODULE(pytypes, m) { m.def("tuple_ssize_t", []() { return py::tuple{(py::ssize_t) 0}; }); m.def("tuple_size_t", []() { return py::tuple{(py::size_t) 0}; }); m.def("get_tuple", []() { return py::make_tuple(42, py::none(), "spam"); }); + m.def("access_tuple_with_int_index", []() { + py::object tpl = py::make_tuple(1, 2); + return tpl[1]; + }); // test_simple_namespace m.def("get_simple_namespace", []() { diff --git a/tests/test_pytypes.py b/tests/test_pytypes.py index 3e9d51a27..4c5d81319 100644 --- a/tests/test_pytypes.py +++ b/tests/test_pytypes.py @@ -152,6 +152,7 @@ def test_tuple(): assert m.tuple_ssize_t() == () assert m.tuple_size_t() == () assert m.get_tuple() == (42, None, "spam") + assert m.access_tuple_with_int_index() == (2) def test_simple_namespace(): From c3a313b0e4094d544046904a45998f021c3da7a3 Mon Sep 17 00:00:00 2001 From: Aaron Gokaslan Date: Tue, 14 Jun 2022 14:21:32 -0400 Subject: [PATCH 02/11] Use rvalue casting --- include/pybind11/pytypes.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h index f7a8600e3..b415d5591 100644 --- a/include/pybind11/pytypes.h +++ b/include/pybind11/pytypes.h @@ -2269,7 +2269,7 @@ iterator object_api::end() const { template template item_accessor object_api::operator[](T &&key) const { - return {derived(), reinterpret_borrow(detail::object_or_cast(std::forward(key)))}; + return {derived(), detail::object_or_cast(std::forward(key)).cast()}; } template obj_attr_accessor object_api::attr(handle key) const { From 9ac9c077661b18d33c05a51907af80849df17477 Mon Sep 17 00:00:00 2001 From: Aaron Gokaslan Date: Tue, 14 Jun 2022 14:29:25 -0400 Subject: [PATCH 03/11] Add template keyword --- include/pybind11/pytypes.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h index b415d5591..0ad206302 100644 --- a/include/pybind11/pytypes.h +++ b/include/pybind11/pytypes.h @@ -2269,7 +2269,7 @@ iterator object_api::end() const { template template item_accessor object_api::operator[](T &&key) const { - return {derived(), detail::object_or_cast(std::forward(key)).cast()}; + return {derived(), detail::object_or_cast(std::forward(key)).template cast()}; } template obj_attr_accessor object_api::attr(handle key) const { From fdc20a8912010672b9958cf1d2143361921c2f7e Mon Sep 17 00:00:00 2001 From: Aaron Gokaslan Date: Tue, 14 Jun 2022 15:27:57 -0400 Subject: [PATCH 04/11] Is the new test the problem? --- tests/test_pytypes.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_pytypes.py b/tests/test_pytypes.py index 4c5d81319..92a45abbf 100644 --- a/tests/test_pytypes.py +++ b/tests/test_pytypes.py @@ -152,7 +152,7 @@ def test_tuple(): assert m.tuple_ssize_t() == () assert m.tuple_size_t() == () assert m.get_tuple() == (42, None, "spam") - assert m.access_tuple_with_int_index() == (2) + #assert m.access_tuple_with_int_index() == (2) def test_simple_namespace(): From 95ed8a6c290030691e9da001c3c7b09ae7f463f5 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Tue, 14 Jun 2022 19:28:35 +0000 Subject: [PATCH 05/11] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- tests/test_pytypes.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_pytypes.py b/tests/test_pytypes.py index 92a45abbf..23fb582d3 100644 --- a/tests/test_pytypes.py +++ b/tests/test_pytypes.py @@ -152,7 +152,7 @@ def test_tuple(): assert m.tuple_ssize_t() == () assert m.tuple_size_t() == () assert m.get_tuple() == (42, None, "spam") - #assert m.access_tuple_with_int_index() == (2) + # assert m.access_tuple_with_int_index() == (2) def test_simple_namespace(): From 4b3f145b94f89a757c8f058950f2119dd217f178 Mon Sep 17 00:00:00 2001 From: Taiju Yamada Date: Wed, 15 Jun 2022 12:52:00 +0900 Subject: [PATCH 06/11] stronger test --- tests/test_pytypes.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_pytypes.cpp b/tests/test_pytypes.cpp index ef937db2f..e9e14ce34 100644 --- a/tests/test_pytypes.cpp +++ b/tests/test_pytypes.cpp @@ -127,8 +127,8 @@ TEST_SUBMODULE(pytypes, m) { m.def("tuple_size_t", []() { return py::tuple{(py::size_t) 0}; }); m.def("get_tuple", []() { return py::make_tuple(42, py::none(), "spam"); }); m.def("access_tuple_with_int_index", []() { - py::object tpl = py::make_tuple(1, 2); - return tpl[1]; + py::object tpl = py::make_tuple(py::make_tuple(1, 2), py::make_tuple(3, 4)); + return tpl[0][1]; }); // test_simple_namespace From 6fa2bb161e331c85ba4c6f79e73642e68dd7a008 Mon Sep 17 00:00:00 2001 From: Taiju Yamada Date: Wed, 15 Jun 2022 13:17:11 +0900 Subject: [PATCH 07/11] actually need both tests --- tests/test_pytypes.cpp | 8 ++++++-- tests/test_pytypes.py | 3 ++- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/tests/test_pytypes.cpp b/tests/test_pytypes.cpp index e9e14ce34..196114745 100644 --- a/tests/test_pytypes.cpp +++ b/tests/test_pytypes.cpp @@ -127,8 +127,12 @@ TEST_SUBMODULE(pytypes, m) { m.def("tuple_size_t", []() { return py::tuple{(py::size_t) 0}; }); m.def("get_tuple", []() { return py::make_tuple(42, py::none(), "spam"); }); m.def("access_tuple_with_int_index", []() { - py::object tpl = py::make_tuple(py::make_tuple(1, 2), py::make_tuple(3, 4)); - return tpl[0][1]; + py::object tpl = py::make_tuple(1, 2); + return tpl[1]; + }); + m.def("access_tuple_with_int_index_multidimension", []() { + py::object tpl = py::make_tuple(py::make_tuple(1, 2, 3), py::make_tuple(3, 4, 5)); + return tpl[1][2]; }); // test_simple_namespace diff --git a/tests/test_pytypes.py b/tests/test_pytypes.py index 23fb582d3..760f6236d 100644 --- a/tests/test_pytypes.py +++ b/tests/test_pytypes.py @@ -152,7 +152,8 @@ def test_tuple(): assert m.tuple_ssize_t() == () assert m.tuple_size_t() == () assert m.get_tuple() == (42, None, "spam") - # assert m.access_tuple_with_int_index() == (2) + assert m.access_tuple_with_int_index() == 2 + assert m.access_tuple_with_int_index_multidimension() == 5 def test_simple_namespace(): From bf238d91ed222c5ade504b3adec5789506cbbdb5 Mon Sep 17 00:00:00 2001 From: Taiju Yamada Date: Wed, 15 Jun 2022 14:10:51 +0900 Subject: [PATCH 08/11] add dict tests --- tests/test_pytypes.cpp | 12 ++++++++++++ tests/test_pytypes.py | 3 +++ 2 files changed, 15 insertions(+) diff --git a/tests/test_pytypes.cpp b/tests/test_pytypes.cpp index 196114745..741a0b53c 100644 --- a/tests/test_pytypes.cpp +++ b/tests/test_pytypes.cpp @@ -120,6 +120,18 @@ TEST_SUBMODULE(pytypes, m) { [](const py::dict &dict, py::object val) { return dict.contains(val); }); m.def("dict_contains", [](const py::dict &dict, const char *val) { return dict.contains(val); }); + m.def("access_dict_with_str", []() { + py::dict d1 = py::dict(); + d1["x"] = 1; + py::object d2 = d1; + return d2["x"]; + }); + m.def("access_dict_with_int", []() { + py::dict d1 = py::dict(); + d1[1] = 1; + py::object d2 = d1; + return d2[1]; + }); // test_tuple m.def("tuple_no_args", []() { return py::tuple{}; }); diff --git a/tests/test_pytypes.py b/tests/test_pytypes.py index 760f6236d..f63134c0c 100644 --- a/tests/test_pytypes.py +++ b/tests/test_pytypes.py @@ -146,6 +146,9 @@ def test_dict(capture, doc): assert m.dict_keyword_constructor() == {"x": 1, "y": 2, "z": 3} + assert m.access_dict_with_str() == 1 + assert m.access_dict_with_int() == 1 + def test_tuple(): assert m.tuple_no_args() == () From c19ac6a55cbd920374d958b087a83121961efa9b Mon Sep 17 00:00:00 2001 From: Taiju Yamada Date: Wed, 15 Jun 2022 15:39:47 +0900 Subject: [PATCH 09/11] perhaps should not touch newly created tuple as object --- tests/test_pytypes.cpp | 10 ++++++---- tests/test_pytypes.py | 5 +++-- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/tests/test_pytypes.cpp b/tests/test_pytypes.cpp index 741a0b53c..b194235d4 100644 --- a/tests/test_pytypes.cpp +++ b/tests/test_pytypes.cpp @@ -138,12 +138,14 @@ TEST_SUBMODULE(pytypes, m) { m.def("tuple_ssize_t", []() { return py::tuple{(py::ssize_t) 0}; }); m.def("tuple_size_t", []() { return py::tuple{(py::size_t) 0}; }); m.def("get_tuple", []() { return py::make_tuple(42, py::none(), "spam"); }); - m.def("access_tuple_with_int_index", []() { - py::object tpl = py::make_tuple(1, 2); + + m.def("access_tuple", [](py::tuple &tpl) { return tpl[1]; }); - m.def("access_tuple_with_int_index_multidimension", []() { - py::object tpl = py::make_tuple(py::make_tuple(1, 2, 3), py::make_tuple(3, 4, 5)); + m.def("access_tuple_as_object_with_int_index", [](py::object &tpl) { + return tpl[1]; + }); + m.def("access_tuple_as_object_with_int_index_multidimension", [](py::object &tpl) { return tpl[1][2]; }); diff --git a/tests/test_pytypes.py b/tests/test_pytypes.py index f63134c0c..4220d17e3 100644 --- a/tests/test_pytypes.py +++ b/tests/test_pytypes.py @@ -155,8 +155,9 @@ def test_tuple(): assert m.tuple_ssize_t() == () assert m.tuple_size_t() == () assert m.get_tuple() == (42, None, "spam") - assert m.access_tuple_with_int_index() == 2 - assert m.access_tuple_with_int_index_multidimension() == 5 + assert m.access_tuple((1,2)) == 2 + assert m.access_tuple_as_object_with_int_index((1,2)) == 2 + assert m.access_tuple_as_object_with_int_index_multidimension(((1,2,3),(4,5,6))) == 6 def test_simple_namespace(): From d77c95d0688609464ce42d5cd47b2869bea6df8a Mon Sep 17 00:00:00 2001 From: Taiju Yamada Date: Wed, 15 Jun 2022 14:58:04 +0900 Subject: [PATCH 10/11] add tests --- tests/test_pytypes.cpp | 26 +++++++++++++++++++++++++- tests/test_pytypes.py | 4 ++++ 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/tests/test_pytypes.cpp b/tests/test_pytypes.cpp index b194235d4..311464163 100644 --- a/tests/test_pytypes.cpp +++ b/tests/test_pytypes.cpp @@ -71,6 +71,20 @@ TEST_SUBMODULE(pytypes, m) { py::print("list item {}: {}"_s.format(index++, item)); } }); + m.def("access_list", []() { + py::list l1 = py::list(); + l1.append(1); + l1.append(2); + return l1[1]; + }); + m.def("access_list_as_object", []() { + py::list l1 = py::list(); + l1.append(1); + l1.append(2); + py::object l2 = l1; + return l2[1]; + }); + // test_none m.def("get_none", [] { return py::none(); }); m.def("print_none", [](const py::none &none) { py::print("none: {}"_s.format(none)); }); @@ -121,12 +135,22 @@ TEST_SUBMODULE(pytypes, m) { m.def("dict_contains", [](const py::dict &dict, const char *val) { return dict.contains(val); }); m.def("access_dict_with_str", []() { + py::dict d1 = py::dict(); + d1["x"] = 1; + return d1["x"]; + }); + m.def("access_dict_with_int", []() { + py::dict d1 = py::dict(); + d1[1] = 1; + return d1[1]; + }); + m.def("access_dict_as_object_with_str", []() { py::dict d1 = py::dict(); d1["x"] = 1; py::object d2 = d1; return d2["x"]; }); - m.def("access_dict_with_int", []() { + m.def("access_dict_as_object_with_int", []() { py::dict d1 = py::dict(); d1[1] = 1; py::object d2 = d1; diff --git a/tests/test_pytypes.py b/tests/test_pytypes.py index 4220d17e3..1905b4532 100644 --- a/tests/test_pytypes.py +++ b/tests/test_pytypes.py @@ -59,6 +59,8 @@ def test_list(capture, doc): assert doc(m.get_list) == "get_list() -> list" assert doc(m.print_list) == "print_list(arg0: list) -> None" + assert m.access_list() == 2 + assert m.access_list_as_object() == 2 def test_none(capture, doc): assert doc(m.get_none) == "get_none() -> None" @@ -148,6 +150,8 @@ def test_dict(capture, doc): assert m.access_dict_with_str() == 1 assert m.access_dict_with_int() == 1 + assert m.access_dict_as_object_with_str() == 1 + assert m.access_dict_as_object_with_int() == 1 def test_tuple(): From ce37acbd6d897f25feeafc8ff5d568f03e674773 Mon Sep 17 00:00:00 2001 From: Taiju Yamada Date: Sun, 19 Jun 2022 19:56:52 +0900 Subject: [PATCH 11/11] code review --- tests/test_pytypes.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/test_pytypes.cpp b/tests/test_pytypes.cpp index 311464163..4fc99fbd0 100644 --- a/tests/test_pytypes.cpp +++ b/tests/test_pytypes.cpp @@ -81,7 +81,7 @@ TEST_SUBMODULE(pytypes, m) { py::list l1 = py::list(); l1.append(1); l1.append(2); - py::object l2 = l1; + py::object l2 = std::move(l1); return l2[1]; }); @@ -147,13 +147,13 @@ TEST_SUBMODULE(pytypes, m) { m.def("access_dict_as_object_with_str", []() { py::dict d1 = py::dict(); d1["x"] = 1; - py::object d2 = d1; + py::object d2 = std::move(d1); return d2["x"]; }); m.def("access_dict_as_object_with_int", []() { py::dict d1 = py::dict(); d1[1] = 1; - py::object d2 = d1; + py::object d2 = std::move(d1); return d2[1]; });