Skip to content
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

Add additional info to TypeError when C++->Python casting fails #3605

Merged
merged 4 commits into from
Jan 11, 2022

Conversation

Skylion007
Copy link
Collaborator

@Skylion007 Skylion007 commented Jan 9, 2022

Description

Adds additional information when raising a type error during function return. Inspired by the unexpected behavior from #3600 .

Suggested changelog entry:

* If possible, attach Python exception with ``py::raise_from`` to TypeError when casting from C++ to Python. This will give additional info if Python exceptions occur in the caster. Adds a test case of trying to convert a set from C++ to Python when the hash function is not defined in Python. 

As a meta note, this is an issue because we are a bit inconsistent in how we handle Python exceptions with py::set, py::dict, etc...). We currently not raising an exception from Python to C++ when interacting with them, maybe we should consider doing that.

Copy link
Collaborator

@rwgk rwgk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Aaron!

tests/test_operator_overloading.cpp Show resolved Hide resolved
tests/test_operator_overloading.py Show resolved Hide resolved
@Skylion007 Skylion007 merged commit ef070f7 into pybind:master Jan 11, 2022
@Skylion007 Skylion007 deleted the raise_from_return_error branch January 11, 2022 02:18
@github-actions github-actions bot added the needs changelog Possibly needs a changelog entry label Jan 11, 2022
@henryiii henryiii removed the needs changelog Possibly needs a changelog entry label Feb 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants