From 71e8a37056d1b171a498de40b717e1c987d05598 Mon Sep 17 00:00:00 2001 From: Paul-Edouard Sarlin Date: Sun, 6 Oct 2024 21:38:03 +0200 Subject: [PATCH 01/10] Print key in map.getitem/delitem KeyError --- include/pybind11/stl_bind.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/pybind11/stl_bind.h b/include/pybind11/stl_bind.h index fcb48dea33..cc696a39a8 100644 --- a/include/pybind11/stl_bind.h +++ b/include/pybind11/stl_bind.h @@ -785,7 +785,7 @@ class_ bind_map(handle scope, const std::string &name, Args && [](Map &m, const KeyType &k) -> MappedType & { auto it = m.find(k); if (it == m.end()) { - throw key_error(); + throw key_error(str(cast(k)).cast()); } return it->second; }, @@ -808,7 +808,7 @@ class_ bind_map(handle scope, const std::string &name, Args && cl.def("__delitem__", [](Map &m, const KeyType &k) { auto it = m.find(k); if (it == m.end()) { - throw key_error(); + throw key_error(str(cast(k)).cast()); } m.erase(it); }); From f2d195148ea56fe60c2d90f492ab598eac8f6ccc Mon Sep 17 00:00:00 2001 From: Paul-Edouard Sarlin Date: Sun, 6 Oct 2024 22:44:51 +0200 Subject: [PATCH 02/10] Add tests --- tests/test_stl_binders.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/test_stl_binders.py b/tests/test_stl_binders.py index 09e784e2eb..c5a386f329 100644 --- a/tests/test_stl_binders.py +++ b/tests/test_stl_binders.py @@ -302,6 +302,14 @@ def test_map_delitem(): assert list(mm) == ["b"] assert list(mm.items()) == [("b", 2.5)] + with pytest.raises(KeyError) as excinfo: + _ = mm["a_long_key"] + assert "a_long_key" in excinfo.value + + with pytest.raises(KeyError) as excinfo: + del mm["a_long_key"] + assert "a_long_key" in excinfo.value + um = m.UnorderedMapStringDouble() um["ua"] = 1.1 um["ub"] = 2.6 From 0a07d1ec52f67879c444d4a2329bae061d757acb Mon Sep 17 00:00:00 2001 From: Paul-Edouard Sarlin Date: Sun, 6 Oct 2024 23:13:09 +0200 Subject: [PATCH 03/10] Fix tests --- tests/test_stl_binders.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_stl_binders.py b/tests/test_stl_binders.py index c5a386f329..08cfa4a485 100644 --- a/tests/test_stl_binders.py +++ b/tests/test_stl_binders.py @@ -304,11 +304,11 @@ def test_map_delitem(): with pytest.raises(KeyError) as excinfo: _ = mm["a_long_key"] - assert "a_long_key" in excinfo.value + assert "a_long_key" in str(excinfo.value) with pytest.raises(KeyError) as excinfo: del mm["a_long_key"] - assert "a_long_key" in excinfo.value + assert "a_long_key" in str(excinfo.value) um = m.UnorderedMapStringDouble() um["ua"] = 1.1 From ce7a05d17dfc11857d95f5542065c92256131127 Mon Sep 17 00:00:00 2001 From: Paul-Edouard Sarlin Date: Sun, 6 Oct 2024 23:37:27 +0200 Subject: [PATCH 04/10] Make robust --- include/pybind11/stl_bind.h | 24 ++++++++++++++++++++++-- tests/test_stl_binders.py | 5 +++++ 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/include/pybind11/stl_bind.h b/include/pybind11/stl_bind.h index cc696a39a8..f1f56c2fa3 100644 --- a/include/pybind11/stl_bind.h +++ b/include/pybind11/stl_bind.h @@ -694,6 +694,26 @@ struct ItemsViewImpl : public detail::items_view { Map ↦ }; +template +std::string format_message_key_error(const KeyType &k) { + std::string message; + try { + message = str(cast(k)); + } catch (const std::exception &) { + try { + message = repr(cast(k)); + } catch (const std::exception &) { + // Leave the message empty. + } + } + const size_t max_length = 80; + if (message.length() > max_length) { + message.resize(max_length); + return message + "..."; + } + return message; +} + PYBIND11_NAMESPACE_END(detail) template , typename... Args> @@ -785,7 +805,7 @@ class_ bind_map(handle scope, const std::string &name, Args && [](Map &m, const KeyType &k) -> MappedType & { auto it = m.find(k); if (it == m.end()) { - throw key_error(str(cast(k)).cast()); + throw key_error(detail::format_message_key_error(k)); } return it->second; }, @@ -808,7 +828,7 @@ class_ bind_map(handle scope, const std::string &name, Args && cl.def("__delitem__", [](Map &m, const KeyType &k) { auto it = m.find(k); if (it == m.end()) { - throw key_error(str(cast(k)).cast()); + throw key_error(detail::format_message_key_error(k)); } m.erase(it); }); diff --git a/tests/test_stl_binders.py b/tests/test_stl_binders.py index 08cfa4a485..f7d4a79aa3 100644 --- a/tests/test_stl_binders.py +++ b/tests/test_stl_binders.py @@ -310,6 +310,11 @@ def test_map_delitem(): del mm["a_long_key"] assert "a_long_key" in str(excinfo.value) + k = "".join(map(str, range(1000))) + with pytest.raises(KeyError) as excinfo: + del mm[k] + assert (k[:80] + "...") in str(excinfo.value) + um = m.UnorderedMapStringDouble() um["ua"] = 1.1 um["ub"] = 2.6 From ce0b107d7abcff528fc5007094e91da358c4fba3 Mon Sep 17 00:00:00 2001 From: Paul-Edouard Sarlin Date: Mon, 7 Oct 2024 00:11:43 +0200 Subject: [PATCH 05/10] Make clang-tidy happy --- include/pybind11/stl_bind.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/pybind11/stl_bind.h b/include/pybind11/stl_bind.h index f1f56c2fa3..91b1b976ea 100644 --- a/include/pybind11/stl_bind.h +++ b/include/pybind11/stl_bind.h @@ -703,7 +703,7 @@ std::string format_message_key_error(const KeyType &k) { try { message = repr(cast(k)); } catch (const std::exception &) { - // Leave the message empty. + return message; } } const size_t max_length = 80; From fcc6bdee5096f174ac964bd2c3d0c96308e5b8a0 Mon Sep 17 00:00:00 2001 From: Paul-Edouard Sarlin Date: Mon, 7 Oct 2024 22:39:39 +0200 Subject: [PATCH 06/10] Return a Python str --- include/pybind11/stl_bind.h | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/include/pybind11/stl_bind.h b/include/pybind11/stl_bind.h index 91b1b976ea..603cabd56c 100644 --- a/include/pybind11/stl_bind.h +++ b/include/pybind11/stl_bind.h @@ -695,21 +695,26 @@ struct ItemsViewImpl : public detail::items_view { }; template -std::string format_message_key_error(const KeyType &k) { - std::string message; +str format_message_key_error(const KeyType &key) { + str message = "pybind11::bind_map key"; + object py_key; try { - message = str(cast(k)); + py_key = cast(key); + } catch (const std::exception &) { + return message; + } + try { + message = str(py_key); } catch (const std::exception &) { try { - message = repr(cast(k)); + message = repr(py_key); } catch (const std::exception &) { return message; } } const size_t max_length = 80; - if (message.length() > max_length) { - message.resize(max_length); - return message + "..."; + if (len(message) > max_length) { + return str(message[slice(0, max_length, 1)]) + str("..."); } return message; } @@ -805,7 +810,8 @@ class_ bind_map(handle scope, const std::string &name, Args && [](Map &m, const KeyType &k) -> MappedType & { auto it = m.find(k); if (it == m.end()) { - throw key_error(detail::format_message_key_error(k)); + set_error(PyExc_KeyError, detail::format_message_key_error(k)); + throw error_already_set(); } return it->second; }, @@ -828,7 +834,8 @@ class_ bind_map(handle scope, const std::string &name, Args && cl.def("__delitem__", [](Map &m, const KeyType &k) { auto it = m.find(k); if (it == m.end()) { - throw key_error(detail::format_message_key_error(k)); + set_error(PyExc_KeyError, detail::format_message_key_error(k)); + throw error_already_set(); } m.erase(it); }); From c3c9e2b957cc4d80cb09a8eab21a674f7ef0a490 Mon Sep 17 00:00:00 2001 From: Paul-Edouard Sarlin Date: Mon, 7 Oct 2024 23:37:16 +0200 Subject: [PATCH 07/10] Show beginning and end of the message --- include/pybind11/stl_bind.h | 5 +++-- tests/test_stl_binders.py | 4 +++- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/include/pybind11/stl_bind.h b/include/pybind11/stl_bind.h index 603cabd56c..d7e8e7f4d8 100644 --- a/include/pybind11/stl_bind.h +++ b/include/pybind11/stl_bind.h @@ -712,9 +712,10 @@ str format_message_key_error(const KeyType &key) { return message; } } - const size_t max_length = 80; + const size_t max_length = 200; if (len(message) > max_length) { - return str(message[slice(0, max_length, 1)]) + str("..."); + return str(message[slice(0, max_length / 2, 1)]) + str("...") + + str(message[slice(len(message) - max_length / 2, len(message), 1)]); } return message; } diff --git a/tests/test_stl_binders.py b/tests/test_stl_binders.py index f7d4a79aa3..ca8941a8d5 100644 --- a/tests/test_stl_binders.py +++ b/tests/test_stl_binders.py @@ -313,7 +313,9 @@ def test_map_delitem(): k = "".join(map(str, range(1000))) with pytest.raises(KeyError) as excinfo: del mm[k] - assert (k[:80] + "...") in str(excinfo.value) + max_length = 200 + k_repr = k[: max_length // 2] + "..." + k[-max_length // 2 :] + assert k_repr in str(excinfo.value) um = m.UnorderedMapStringDouble() um["ua"] = 1.1 From 917226f4c9c09dddf9c7a1358b45631b287d0ca0 Mon Sep 17 00:00:00 2001 From: Paul-Edouard Sarlin Date: Mon, 7 Oct 2024 23:50:38 +0200 Subject: [PATCH 08/10] Avoid implicit conversion --- include/pybind11/stl_bind.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/include/pybind11/stl_bind.h b/include/pybind11/stl_bind.h index d7e8e7f4d8..759047897a 100644 --- a/include/pybind11/stl_bind.h +++ b/include/pybind11/stl_bind.h @@ -712,10 +712,10 @@ str format_message_key_error(const KeyType &key) { return message; } } - const size_t max_length = 200; - if (len(message) > max_length) { - return str(message[slice(0, max_length / 2, 1)]) + str("...") - + str(message[slice(len(message) - max_length / 2, len(message), 1)]); + const ssize_t half_max_length = 100; + if (len(message) > half_max_length * 2) { + return str(message[slice(0, half_max_length, 1)]) + str("...") + + str(message[slice(-half_max_length, static_cast(len(message)), 1)]); } return message; } From ef1774315db59797ed8183b90dd0230bddb8efba Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Mon, 7 Oct 2024 15:38:24 -0700 Subject: [PATCH 09/10] Split out `format_message_key_error_key_object()` to reduce amount of templated code. --- include/pybind11/stl_bind.h | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/include/pybind11/stl_bind.h b/include/pybind11/stl_bind.h index 759047897a..21bb4cf5e2 100644 --- a/include/pybind11/stl_bind.h +++ b/include/pybind11/stl_bind.h @@ -694,13 +694,9 @@ struct ItemsViewImpl : public detail::items_view { Map ↦ }; -template -str format_message_key_error(const KeyType &key) { +inline str format_message_key_error_key_object(handle py_key) { str message = "pybind11::bind_map key"; - object py_key; - try { - py_key = cast(key); - } catch (const std::exception &) { + if (!py_key) { return message; } try { @@ -720,6 +716,18 @@ str format_message_key_error(const KeyType &key) { return message; } +template +str format_message_key_error(const KeyType &key) { + object py_key; + try { + py_key = cast(key); + } catch (const std::exception &) { + do { // Trick to avoid "empty catch" warning/error. + } while (false); + } + return format_message_key_error_key_object(py_key); +} + PYBIND11_NAMESPACE_END(detail) template , typename... Args> From f80d132a79c7222096015988bff90de86e4ea9e4 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Tue, 8 Oct 2024 09:43:20 -0700 Subject: [PATCH 10/10] =?UTF-8?q?Use=20`"=E2=9C=84=E2=9C=84=E2=9C=84"`=20i?= =?UTF-8?q?nstead=20of=20`"..."`?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Also rename variable to `cut_length`, to not get into even/odd issues with the meaning of "half". --- include/pybind11/stl_bind.h | 8 ++++---- tests/test_stl_binders.py | 14 +++++++++----- 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/include/pybind11/stl_bind.h b/include/pybind11/stl_bind.h index 21bb4cf5e2..af3a47f39c 100644 --- a/include/pybind11/stl_bind.h +++ b/include/pybind11/stl_bind.h @@ -708,10 +708,10 @@ inline str format_message_key_error_key_object(handle py_key) { return message; } } - const ssize_t half_max_length = 100; - if (len(message) > half_max_length * 2) { - return str(message[slice(0, half_max_length, 1)]) + str("...") - + str(message[slice(-half_max_length, static_cast(len(message)), 1)]); + const ssize_t cut_length = 100; + if (len(message) > 2 * cut_length + 3) { + return str(message[slice(0, cut_length, 1)]) + str("✄✄✄") + + str(message[slice(-cut_length, static_cast(len(message)), 1)]); } return message; } diff --git a/tests/test_stl_binders.py b/tests/test_stl_binders.py index ca8941a8d5..9856ba462b 100644 --- a/tests/test_stl_binders.py +++ b/tests/test_stl_binders.py @@ -303,18 +303,22 @@ def test_map_delitem(): assert list(mm.items()) == [("b", 2.5)] with pytest.raises(KeyError) as excinfo: - _ = mm["a_long_key"] + mm["a_long_key"] assert "a_long_key" in str(excinfo.value) with pytest.raises(KeyError) as excinfo: del mm["a_long_key"] assert "a_long_key" in str(excinfo.value) - k = "".join(map(str, range(1000))) + cut_length = 100 + k_very_long = "ab" * cut_length + "xyz" with pytest.raises(KeyError) as excinfo: - del mm[k] - max_length = 200 - k_repr = k[: max_length // 2] + "..." + k[-max_length // 2 :] + mm[k_very_long] + assert k_very_long in str(excinfo.value) + k_very_long += "@" + with pytest.raises(KeyError) as excinfo: + mm[k_very_long] + k_repr = k_very_long[:cut_length] + "✄✄✄" + k_very_long[-cut_length:] assert k_repr in str(excinfo.value) um = m.UnorderedMapStringDouble()