Cannot return option from PySequenceProtocol __getitem__

[pattonw]

🐛 Bug Reports

When reporting a bug, please provide the following information. If this is not a bug report you can just discard this template.

🌍 Environment

• Your operating system and version: Ubuntu 18.04.4
• Your python version: 3.7.5
• How did you install python (e.g. apt or pyenv)? Did you use a virtualenv?: virtualenv
• Your rust version (rustc --version): rustc 1.43.0-nightly (18c275b42 2020-03-02)
• Are you using the latest pyo3 version? Have you tried using latest master (replace version = "0.x.y" with git = "https://github.com/PyO3/pyo3")? Yes, using latest master

💥 Reproducing

lib.rs

use pyo3::class::PySequenceProtocol;
use pyo3::prelude::*;
use pyo3::{exceptions, PyErr, PyResult};

#[pyclass(module = "test", dict)]
#[derive(Clone, Debug)]
struct Test {
    #[pyo3(get, set)]
    value: Vec<Option<i64>>,
}

#[pyproto]
impl PySequenceProtocol for Test {
    fn __getitem__(&self, idx: isize) -> PyResult<Option<i64>> {
        match self.value.get(idx as usize) {
            Some(x) => Ok(*x),
            None => Err(PyErr::new::<exceptions::IndexError, _>(
                "Index out of bounds",
            )),
        }
    }
}

Problem:
"the trait bound std::result::Result<i64, pyo3::err::PyErr>: std::convert::From<std::result::Result<std::option::Option<i64>, pyo3::err::PyErr>> is not satisfied"

I don't want to convert from std::result::Result<std::option::Option<i64>, pyo3::err::PyErr> to std::result::Result<i64, pyo3::err::PyErr>. I want to return an std::result::Result<Option<i64>, pyo3::err::PyErr>

[pattonw]

Similar errors seem to arise in other places such as using an option in __setitem__ or __contains__. Iteresting to note that __next__ in PyIterProtocol is happy returning a PyResult<Option<Option<i64>>>

[kngwyu]

Thank you for filing!

[kngwyu]

This is because we treat PyResult<Option<T>> specially to make __next__ work.
So an immediate fix can be possible by, say, only doing this hack for __next__.
But iterator has some other problems and I feel we need an alternative of Into<PyResult<...>> ... 🤔

[davidhewitt]

I feel we need an alternative of Into<PyResult<...>> ... 🤔

I wonder if we can use IntoPyCallbackOutput trait instead? Possibly with a trick like LenCallbackOutput for __next__ to solve the problem there.

Give me a chance to play around with a PR for this later tonight 😄

[davidhewitt]

So I took a look tonight, it's not super easy but I think a solution is possible using the callback conversion code, though it might need a little refactoring. I'll continue to play through the week, not sure when a PR would be ready.

[davidhewitt]

Just a final note on this having explored it further - any solution will need quite a few changes, so I don't think you should feel you have to wait for this for 0.11 release.

[kngwyu]

What I meant as the problem of the current PyIterProtocol is, as I said in the comment, it cannot returns the value.
I.e., we can't express:

def iter_():
    for i in range(5):
        yield i;
    return "Ended"

So __next__ actually have to return Into<PyResult<Either<Success, Return>>>, instead of Option.
However, I feel it's silly to do a breaking change for such a minor feature.
To avoid this breaking change, the return type can be either Either or Option, but I'm not sure we can express it by just an Into trait.

I wonder if we can use IntoPyCallbackOutput trait instead?

Using a custom callback converter could be a good idea, but I'm not sure it is a general solution.