Misleading error: Missing trait IntoPyCallbackOutput

[mejrs]

First of all, great crate :)

Just now, I was trying to get this pyclass/ Rust struct to compile. It's basically a struct around a hashmap that I want to expose a Python .get method for.

#[pyclass]
#[derive(Debug, Clone)]
pub struct ParamTable {
    #[doc(hidden)]
    pub params: HashMap<u32, Param>,
}

#[pymethods]
impl ParamTable{
	pub fn get(&self, id: u32) -> PyResult<Option<&Param>>{
		Ok(self.params.get(&id))
	}
}

#[derive(Debug, Eq, PartialEq, Clone)]
pub enum Param {
    Integer(i32),
    String(String),
}

This results in the following compilation error:

error[E0277]: the trait bound `Result<std::option::Option<&structures::Param>, PyErr>: IntoPyCallbackOutput<_>` is not satisfied
   --> src\cache\structures.rs:37:1
    |
37  | #[pymethods]
    | ^^^^^^^^^^^^ the trait `IntoPyCallbackOutput<_>` is not implemented for `Result<std::option::Option<&structures::Param>, PyErr>`
    |
   ::: C:\Users\bruno\.cargo\registry\src\github.com-1ecc6299db9ec823\pyo3-0.13.2\src\callback.rs:170:8
    |
170 |     T: IntoPyCallbackOutput<U>,
    |        ----------------------- required by this bound in `pyo3::callback::convert`
    |
    = help: the following implementations were found:
              <Result<T, E> as IntoPyCallbackOutput<U>>
    = note: this error originates in an attribute macro (in Nightly builds, run with -Z macro-backtrace for more info)

Sadly, IntoPyCallbackOutput has no documentation and can't be found in the user guide.

I eventually figured out that I needed to implement IntoPy:

impl IntoPy<PyObject> for &Param {
    fn into_py(self, py: Python) -> PyObject {
        match self{
			Param::Integer(val) => val.into_py(py),
			Param::String(val) => val.into_py(py),
		}
    }
}

Is this something that can be fixed? It did take me a while to figure out.

[davidhewitt]

At the moment we rely on the IntoPyCallbackOutput trait internally (it deals with the fact that types may or may not be wrapped in Result, among with other things).

At the moment this trait is an implementation detail that we don't want users to implement themselves, so it uses #[doc(hidden)].

We could add docs for this trait with a big label saying to implement this trait, implement IntoPy<PyObject>. We'd like to keep it possible that we can change this trait if needed in the future. It's purely an implementation detail.

[mejrs]

I am interested in implementing this myself.

We could add docs for this trait with a big label saying to implement this trait, implement IntoPy

I think it would be better to make a doc alias for it on IntoPy, so it redirects there when you search the documentation for IntoPyCallbackOutput.

Also the documentation for IntoPy is pretty bare; I plan to expand on that as well.

[davidhewitt]

Thanks, all help with implementing documentation is much appreciated!

#[doc(alias = "...")] on IntoPy would be cool, but it's not supported by our minimum rustc version, so you'd have to do some kind of dance with #[cfg_attr] and build.rs to conditionally enable it. It may be simpler to add a small doc on IntoPyCallbackOutput leading the reader to IntoPy; up to you.

[mejrs]

We can use #[rustversion::attr(since(1.48), doc(alias = "IntoPyCallbackOutput"))].

That is, if you are okay with adding rustversion as a dependency (it's currently a dev-dependency).

[davidhewitt]

It's nice to avoid the extra dependency if we can... Perhaps use the same trick we're introducing in #1128 to check the rust version in build.rs and add a cfg?