rust-cpython icon indicating copy to clipboard operation
rust-cpython copied to clipboard

Implement Display and Debug traits

Open jared-mackey opened this issue 9 years ago • 4 comments

These traits are extremely useful and would be great for debugging issues.

jared-mackey avatar Oct 14 '16 02:10 jared-mackey

Display and Debug are already implemented for PyObject. For which type(s) are looking for an implementation?

As an aside, I'm not sure I like the way these traits are currently implemented. Display calls Python str() and Debug calls Python repr(). This is fine when everything works as expected, but:

  • Because the Display/Debug traits don't accept a Python parameter, these impls have to silently acquire the GIL, introducing some risk of deadlocks.
  • The code doesn't correctly handle the case where str()/repr() throws a python exception.

So I'm thinking maybe Display shouldn't be implemented after all, and Debug should just output something like PyObject(0x12345678) (address of python object). But that's not very useful compared to the repr() output...

dgrunwald avatar Oct 15 '16 09:10 dgrunwald

I personally think having it for all types would be nice for debugging purposes. But having risks of deadlocks for printing is a problem most people wouldn't expect so we would need to figure out a way around that.

jared-mackey avatar Oct 26 '16 05:10 jared-mackey

We could add a to_repr method that takes in the Gil as a parameter and just returns a String of the objects repr.

For example

println!("{}", data_dict.to_repr(py));

where data_dict is a PyDict instance.

jared-mackey avatar Oct 26 '16 05:10 jared-mackey

I personally think having it for all types would be nice for debugging purposes.

I agree this comment even if the output is like PyObject(0x12345678). I found Debug and Display traits are not implemented on python3-sys::PyObject and other types in FFI. I am trying to write wrapper of NumPy C-API using python3-sys crate, and the lack of Debug trait makes me wondering. Is there any reason not to implement them?

termoshtt avatar Apr 21 '17 05:04 termoshtt