-
Notifications
You must be signed in to change notification settings - Fork 2.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[smart_holder] Change throw_if_uninitialized_or_disowned_holder()
to also show C++ type name.
#4977
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||
---|---|---|---|---|---|---|---|---|---|---|
|
@@ -9,23 +9,19 @@ | |||||||||
|
||||||||||
@pytest.mark.xfail("env.PYPY", reason="gc after `del field` is apparently deferred") | ||||||||||
@pytest.mark.parametrize("m_attr", ["m_valu_readonly", "m_valu_readwrite"]) | ||||||||||
def test_valu_getter(msg, m_attr): | ||||||||||
def test_valu_getter(m_attr): | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: this function name is apparently misspelled There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's intentional, see comment at top of this file, which points here: pybind11/tests/test_class_sh_property.cpp Lines 1 to 4 in feed8b1
(The original idea was actually to make this code in test_class_sh_basic.cpp more readable.) |
||||||||||
# Reduced from PyCLIF test: | ||||||||||
# https://github.com/google/clif/blob/c371a6d4b28d25d53a16e6d2a6d97305fb1be25a/clif/testing/python/nested_fields_test.py#L56 | ||||||||||
outer = m.Outer() | ||||||||||
field = getattr(outer, m_attr) | ||||||||||
assert field.num == -99 | ||||||||||
with pytest.raises(ValueError) as excinfo: | ||||||||||
m.DisownOuter(outer) | ||||||||||
assert msg(excinfo.value) == "Cannot disown use_count != 1 (loaded_as_unique_ptr)." | ||||||||||
assert str(excinfo.value) == "Cannot disown use_count != 1 (loaded_as_unique_ptr)." | ||||||||||
del field | ||||||||||
m.DisownOuter(outer) | ||||||||||
with pytest.raises(ValueError) as excinfo: | ||||||||||
with pytest.raises(ValueError, match="Python instance was disowned") as excinfo: | ||||||||||
getattr(outer, m_attr) | ||||||||||
assert ( | ||||||||||
msg(excinfo.value) | ||||||||||
== "Missing value for wrapped C++ type: Python instance was disowned." | ||||||||||
) | ||||||||||
|
||||||||||
|
||||||||||
def test_valu_setter(): | ||||||||||
|
@@ -85,18 +81,14 @@ def test_ptr(field_type, num_default, outer_type, m_attr, r_kind): | |||||||||
|
||||||||||
|
||||||||||
@pytest.mark.parametrize("m_attr_readwrite", ["m_uqmp_readwrite", "m_uqcp_readwrite"]) | ||||||||||
def test_uqp(m_attr_readwrite, msg): | ||||||||||
def test_uqp(m_attr_readwrite): | ||||||||||
outer = m.Outer() | ||||||||||
assert getattr(outer, m_attr_readwrite) is None | ||||||||||
field_orig = m.Field() | ||||||||||
field_orig.num = 39 | ||||||||||
setattr(outer, m_attr_readwrite, field_orig) | ||||||||||
with pytest.raises(ValueError) as excinfo: | ||||||||||
with pytest.raises(ValueError, match="Python instance was disowned"): | ||||||||||
_ = field_orig.num | ||||||||||
assert ( | ||||||||||
msg(excinfo.value) | ||||||||||
== "Missing value for wrapped C++ type: Python instance was disowned." | ||||||||||
) | ||||||||||
field_retr1 = getattr(outer, m_attr_readwrite) | ||||||||||
assert getattr(outer, m_attr_readwrite) is None | ||||||||||
assert field_retr1.num == 39 | ||||||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I dont' suppose you can use f-strings?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting idea, but no. We don't have f-strings in C++ (I never thought about that before, not sure how feasible that is), and compared to relying on
std::string
operator+
, the overhead of using the Python C API is not something I'd want to have in an error handling code path like here.