Skip to content

Commit 56e69a2

Browse files
sarlinperwgk
andauthored
Print key in KeyError in map.__getitem__/__delitem__ (#5397)
* Print key in map.getitem/delitem KeyError * Add tests * Fix tests * Make robust * Make clang-tidy happy * Return a Python str * Show beginning and end of the message * Avoid implicit conversion * Split out `format_message_key_error_key_object()` to reduce amount of templated code. * Use `"✄✄✄"` instead of `"..."` Also rename variable to `cut_length`, to not get into even/odd issues with the meaning of "half". --------- Co-authored-by: Ralf W. Grosse-Kunstleve <[email protected]>
1 parent c4a05f9 commit 56e69a2

File tree

2 files changed

+57
-2
lines changed

2 files changed

+57
-2
lines changed

include/pybind11/stl_bind.h

Lines changed: 38 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -694,6 +694,40 @@ struct ItemsViewImpl : public detail::items_view {
694694
Map &map;
695695
};
696696

697+
inline str format_message_key_error_key_object(handle py_key) {
698+
str message = "pybind11::bind_map key";
699+
if (!py_key) {
700+
return message;
701+
}
702+
try {
703+
message = str(py_key);
704+
} catch (const std::exception &) {
705+
try {
706+
message = repr(py_key);
707+
} catch (const std::exception &) {
708+
return message;
709+
}
710+
}
711+
const ssize_t cut_length = 100;
712+
if (len(message) > 2 * cut_length + 3) {
713+
return str(message[slice(0, cut_length, 1)]) + str("✄✄✄")
714+
+ str(message[slice(-cut_length, static_cast<ssize_t>(len(message)), 1)]);
715+
}
716+
return message;
717+
}
718+
719+
template <typename KeyType>
720+
str format_message_key_error(const KeyType &key) {
721+
object py_key;
722+
try {
723+
py_key = cast(key);
724+
} catch (const std::exception &) {
725+
do { // Trick to avoid "empty catch" warning/error.
726+
} while (false);
727+
}
728+
return format_message_key_error_key_object(py_key);
729+
}
730+
697731
PYBIND11_NAMESPACE_END(detail)
698732

699733
template <typename Map, typename holder_type = std::unique_ptr<Map>, typename... Args>
@@ -785,7 +819,8 @@ class_<Map, holder_type> bind_map(handle scope, const std::string &name, Args &&
785819
[](Map &m, const KeyType &k) -> MappedType & {
786820
auto it = m.find(k);
787821
if (it == m.end()) {
788-
throw key_error();
822+
set_error(PyExc_KeyError, detail::format_message_key_error(k));
823+
throw error_already_set();
789824
}
790825
return it->second;
791826
},
@@ -808,7 +843,8 @@ class_<Map, holder_type> bind_map(handle scope, const std::string &name, Args &&
808843
cl.def("__delitem__", [](Map &m, const KeyType &k) {
809844
auto it = m.find(k);
810845
if (it == m.end()) {
811-
throw key_error();
846+
set_error(PyExc_KeyError, detail::format_message_key_error(k));
847+
throw error_already_set();
812848
}
813849
m.erase(it);
814850
});

tests/test_stl_binders.py

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -302,6 +302,25 @@ def test_map_delitem():
302302
assert list(mm) == ["b"]
303303
assert list(mm.items()) == [("b", 2.5)]
304304

305+
with pytest.raises(KeyError) as excinfo:
306+
mm["a_long_key"]
307+
assert "a_long_key" in str(excinfo.value)
308+
309+
with pytest.raises(KeyError) as excinfo:
310+
del mm["a_long_key"]
311+
assert "a_long_key" in str(excinfo.value)
312+
313+
cut_length = 100
314+
k_very_long = "ab" * cut_length + "xyz"
315+
with pytest.raises(KeyError) as excinfo:
316+
mm[k_very_long]
317+
assert k_very_long in str(excinfo.value)
318+
k_very_long += "@"
319+
with pytest.raises(KeyError) as excinfo:
320+
mm[k_very_long]
321+
k_repr = k_very_long[:cut_length] + "✄✄✄" + k_very_long[-cut_length:]
322+
assert k_repr in str(excinfo.value)
323+
305324
um = m.UnorderedMapStringDouble()
306325
um["ua"] = 1.1
307326
um["ub"] = 2.6

0 commit comments

Comments
 (0)