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

pyerr: improve debug & display impls #1275

Merged
merged 1 commit into from
Nov 15, 2020

Conversation

davidhewitt
Copy link
Member

This PR aims to improve the ergonomics of PyErr's debug output.

Now, instead of PyErr { type: 0x... } the repr of type and value are printed (using Python::with_gil to acquire the GIL if needed). We already acquire the GIL in the Display impl so I think we have acceptable precedent to do this. It also makes the output much more useful for PyO3 users.

As a simplification, I also removed the special impl Debug and impl Display for PyException. They now just use the underlying Python object's .repr() and .str(), just like PyAny. I think there's a strong consistency argument for doing this.

Before:

PyErr debug         = PyErr { type: 0x7ff6f7b50420 }
PyErr display       = ValueError: bar
PyException debug   = ValueError
PyException display = ValueError: bar
PyAny debug         = ValueError('bar')
PyAny display       = bar

After:

PyErr debug         = PyErr { type: <class 'ValueError'>, value: ValueError('bar'), traceback: None }
PyErr display       = ValueError: bar
PyException debug   = ValueError('bar')
PyException display = bar
PyAny debug         = ValueError('bar')
PyAny display       = bar

@davidhewitt davidhewitt force-pushed the pyerr-debug branch 2 times, most recently from 8ea935d to f8df950 Compare November 14, 2020 18:07
Copy link
Member

@kngwyu kngwyu left a comment

Choose a reason for hiding this comment

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

Awesome 💯

src/err/mod.rs Outdated
let err = py
.run("raise Exception('banana')", None, None)
.expect_err("raising should have given us an error");
assert!(
Copy link
Member

Choose a reason for hiding this comment

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

Looks this assertion fails at Python 3.6. Maybe you can just weaken the assertion.

Copy link
Member Author

Choose a reason for hiding this comment

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

👍 modified slightly to check the individual fields and use #[cfg(Py_3_7)] on the bit that's different

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.

2 participants