-
Notifications
You must be signed in to change notification settings - Fork 783
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 does not implement std::error::Error #682
Comments
Thank you for the proposal.
I'm not sure they are better, but possible alternatives are
PyErr has already the same memory-management semantics as |
To make FWIW I feel it is fine to have multiple ways to do the same thing in pyo3. It is a necessity, in fact, due to the fact that some conversions are not easily possible (such as going from Unless I misunderstand them, options 3&4 are insufficient as they lose information: the type, traceback and causation chain ( |
So how about separating Rust-originated errors into another struct? struct PyErrRust(Box<dyn IntoPyErrorTrait>);
struct PyErr {
ptype: Py<PyType>,
pvalue: PyObject,
ptracebacj: Option<PyObject>,
} |
Yeah, that’s definitely something that can work, the suggested I do think that it is a waste of space to store |
I don't have a better proposal right now but I wanted to voice my concern about the current error handling in pyo3 and my interest in this issue. Right now, I can't seem to be able to fetch the actual python error string in a nice way that I can integrate with my application. Thanks for working on this! P.S: Is there an example that shows how to deal with errors right now? How should python tracebacks be extracted, etc? |
There isn’t an example right now, but if you consider that you need to essentially write the python to extract the information there might be some relevant python tutorials. Extracting tracebacks will never integrate well, sadly, but we could at least add a method to obtain it and implement nice printing. |
I've written some python to extract the traceback but it doesn't seem to work. Basically, call the traceback module when an error happens. I might be doing something wrong, tho. Thanks for the tip |
I have a PR for this almost ready which I'd love get done for |
Hi. I'm currently using the Is the plan to allow these nice errors but only with use pyo3::prelude::*;
#[macro_export]
macro_rules! pytry {
($py:expr, $e:expr) => {{
match $e {
Ok(v) => v,
Err(e) => color_eyre::eyre::bail!("{:?}", e.print($py)),
}
}};
}
fn main() -> Result<(), color_eyre::Report> {
let gil = Python::acquire_gil();
let py = gil.python();
let _ = pytry!(py, PyModule::import(py, "matplotli"));
Ok(())
} My first attempt at this macro was use pyo3::prelude::*;
#[macro_export]
macro_rules! pytry0 {
($e:expr) => {{
match $e {
Ok(v) => v,
Err(e) => color_eyre::eyre::bail!("{:?}", e),
}
}};
}
fn main() -> Result<(), color_eyre::Report> {
let gil = Python::acquire_gil();
let py = gil.python();
let _ = pytry0!(PyModule::import(py, "matplotli"));
Ok(())
} |
Having access to I've wondered about the idea of |
Thanks @davidhewitt. I took a quick look at While digging through the code I've also learned about |
Glad to hear you've found a reasonable compromise here. All feedback like this is very useful. |
Perhaps. At the moment I believe I do agree though that some improvements to the pyo3 API could definitely be made in this space. We'll figure out the right design eventually! |
PyErr is a very odd structure. It pretends to emulate the 3-tuple returned by
PyErr_Fetch
, but then itspvalue
is a 3-variant that also allows lazy construction of exception objects.As thus it fails to implement a couple of traits typically expected of the error type (
std::error::Error
,std::fmt::Display
,Sync
andSend
) and does not integrate very well with the error handling story in Python.There are a couple of ways to go forward here, but one of the simpler things that could be done I think is:
fn(Python, PyErr) -> Py<BaseException>
;Py<ExceptionT>
types.Display
and friends I think it is fine to just do aPython::acquire_gil()
inside if that is necessary to format the data.Willing to contribute the necessary code to implement this. Please let me know what you think about this. Perhaps other approaches seem better?
The text was updated successfully, but these errors were encountered: