Allow other Result types in #[pyfunction] (#1118)

* Added a couple basic tests

* Implemented suggested change

* Fixed type inference

* cargo fmt

* Finished tests and removed warnings

* Include in CHANGELOG.md

* Moved test into separate file

* &'static str and function rename

* Mention in the book

CHANGELOG.md

@@ -13,6 +13,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 - Add `PyBytes::new_with` and `PyByteArray::new_with` for initialising Python-allocated bytes and bytearrays using a closure. [#1074](https://github.com/PyO3/pyo3/pull/1074)
 - Add `Py::as_ref` and `Py::into_ref`. [#1098](https://github.com/PyO3/pyo3/pull/1098)
 - Add optional implementations of `ToPyObject`, `IntoPy`, and `FromPyObject` for [hashbrown](https://crates.io/crates/hashbrown)'s `HashMap` and `HashSet` types. The `hashbrown` feature must be enabled for these implementations to be built. [#1114](https://github.com/PyO3/pyo3/pull/1114/)
+- Allow other `Result` types when using `#[pyfunction]`. [#1106](https://github.com/PyO3/pyo3/issues/1106).
 
 ### Changed
 - Exception types have been renamed from e.g. `RuntimeError` to `PyRuntimeError`, and are now only accessible by `&T` or `Py<T>` similar to other Python-native types. The old names continue to exist but are deprecated. [#1024](https://github.com/PyO3/pyo3/pull/1024)

guide/src/conversions.md

@@ -72,7 +72,7 @@ When returning values from functions callable from Python, Python-native types (
 
 Because these types are references, in some situations the Rust compiler may ask for lifetime annotations. If this is the case, you should use `Py<PyAny>`, `Py<PyDict>` etc. instead - which are also zero-cost. For all of these Python-native types `T`, `Py<T>` can be created from `T` with an `.into()` conversion.
 
-If your function is fallible, it should return `PyResult<T>`, which will raise a `Python` exception if the `Err` variant is returned.
+If your function is fallible, it should return `PyResult<T>` or `Result<T, E>` where `E` implements `From<E> for PyErr`. This will raise a `Python` exception if the `Err` variant is returned.
 
 Finally, the following Rust types are also able to convert to Python as return values:
 

src/callback.rs

@@ -2,7 +2,7 @@
 
 //! Utilities for a Python callable object that invokes a Rust function.
 
-use crate::err::PyResult;
+use crate::err::{PyErr, PyResult};
 use crate::exceptions::PyOverflowError;
 use crate::ffi::{self, Py_hash_t};
 use crate::IntoPyPointer;
@@ -37,12 +37,13 @@ pub trait IntoPyCallbackOutput<Target> {
     fn convert(self, py: Python) -> PyResult<Target>;
 }
 
-impl<T, U> IntoPyCallbackOutput<U> for PyResult<T>
+impl<T, E, U> IntoPyCallbackOutput<U> for Result<T, E>
 where
     T: IntoPyCallbackOutput<U>,
+    E: Into<PyErr>,
 {
     fn convert(self, py: Python) -> PyResult<U> {
-        self.and_then(|t| t.convert(py))
+        self.map_err(Into::into).and_then(|t| t.convert(py))
     }
 }
 

src/class/macros.rs

@@ -152,7 +152,7 @@ macro_rules! py_binary_self_func {
                 let arg = py.from_borrowed_ptr::<$crate::PyAny>(arg);
                 call_operator_mut!(py, slf_, $f, arg).convert(py)?;
                 ffi::Py_INCREF(slf);
-                Ok(slf)
+                Ok::<_, $crate::err::PyErr>(slf)
             })
         }
         Some(wrap::<$class>)

src/class/number.rs

@@ -4,6 +4,7 @@
 //! Trait and support implementation for implementing number protocol
 
 use crate::callback::IntoPyCallbackOutput;
+use crate::err::PyErr;
 use crate::{ffi, FromPyObject, PyClass, PyObject};
 
 /// Number interface
@@ -941,7 +942,7 @@ impl ffi::PyNumberMethods {
                 let other = py.from_borrowed_ptr::<crate::PyAny>(other);
                 call_operator_mut!(py, slf_cell, __ipow__, other).convert(py)?;
                 ffi::Py_INCREF(slf);
-                Ok(slf)
+                Ok::<_, PyErr>(slf)
             })
         }
         self.nb_inplace_power = Some(wrap_ipow::<T>);

tests/test_compile_error.rs

@@ -10,6 +10,7 @@ fn test_compile_errors() {
     t.compile_fail("tests/ui/missing_clone.rs");
     t.compile_fail("tests/ui/reject_generics.rs");
     t.compile_fail("tests/ui/wrong_aspyref_lifetimes.rs");
+    t.compile_fail("tests/ui/invalid_result_conversion.rs");
 
     skip_min_stable(&t);
 

tests/test_various.rs

@@ -3,6 +3,8 @@ use pyo3::types::IntoPyDict;
 use pyo3::types::{PyDict, PyTuple};
 use pyo3::{py_run, wrap_pyfunction, PyCell};
 
+use std::fmt;
+
 mod common;
 
 #[pyclass]
@@ -168,3 +170,41 @@ fn test_pickle() {
     "#
     );
 }
+
+/// Testing https://github.com/PyO3/pyo3/issues/1106. A result type that
+/// implements `From<MyError> for PyErr` should be automatically converted
+/// when using `#[pyfunction]`.
+///
+/// This only makes sure that valid `Result` types do work. For an invalid
+/// enum type, see `ui/invalid_result_conversion.py`.
+#[derive(Debug)]
+struct MyError {
+    pub descr: &'static str,
+}
+
+impl fmt::Display for MyError {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        write!(f, "My error message: {}", self.descr)
+    }
+}
+
+/// Important for the automatic conversion to `PyErr`.
+impl From<MyError> for PyErr {
+    fn from(err: MyError) -> pyo3::PyErr {
+        pyo3::exceptions::PyOSError::py_err(err.to_string())
+    }
+}
+
+#[pyfunction]
+fn result_conversion_function() -> Result<(), MyError> {
+    Err(MyError {
+        descr: "something went wrong",
+    })
+}
+
+#[test]
+fn test_result_conversion() {
+    let gil = Python::acquire_gil();
+    let py = gil.python();
+    wrap_pyfunction!(result_conversion_function)(py);
+}

tests/ui/invalid_result_conversion.rs

@@ -0,0 +1,31 @@
+//! Testing https://github.com/PyO3/pyo3/issues/1106. A result type that
+//! *doesn't* implement `From<MyError> for PyErr` won't be automatically
+//! converted when using `#[pyfunction]`.
+use pyo3::prelude::*;
+use pyo3::wrap_pyfunction;
+
+use std::fmt;
+
+/// A basic error type for the tests. It's missing `From<MyError> for PyErr`,
+/// though, so it shouldn't work.
+#[derive(Debug)]
+struct MyError {
+    pub descr: &'static str,
+}
+
+impl fmt::Display for MyError {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        write!(f, "My error message: {}", self.descr)
+    }
+}
+
+#[pyfunction]
+fn should_not_work() -> Result<(), MyError> {
+    Err(MyError { descr: "something went wrong" })
+}
+
+fn main() {
+    let gil = Python::acquire_gil();
+    let py = gil.python();
+    wrap_pyfunction!(should_not_work)(py);
+}

tests/ui/invalid_result_conversion.stderr

@@ -0,0 +1,14 @@
+error[E0277]: the trait bound `std::result::Result<(), MyError>: pyo3::callback::IntoPyCallbackOutput<_>` is not satisfied
+   --> $DIR/invalid_result_conversion.rs:22:1
+    |
+22  | #[pyfunction]
+    | ^^^^^^^^^^^^^ the trait `pyo3::callback::IntoPyCallbackOutput<_>` is not implemented for `std::result::Result<(), MyError>`
+    |
+   ::: $WORKSPACE/src/callback.rs
+    |
+    |     T: IntoPyCallbackOutput<U>,
+    |        ----------------------- required by this bound in `pyo3::callback::convert`
+    |
+    = help: the following implementations were found:
+              <std::result::Result<T, E> as pyo3::callback::IntoPyCallbackOutput<U>>
+    = note: this error originates in an attribute macro (in Nightly builds, run with -Z macro-backtrace for more info)