Clarified documentation for implementing iteration.

[abingham]

Added a note about implementing the distinction between iterables and iterators. I stumbled over this a bit when I was implementing some bindings, so I thought some clarification would help.

[kngwyu]

Good catch, thank you!
But generally, we don't need to implement __next__ for containers.
To clarify the relationship between container and iterator, I propose the below example.

#[pyclass]
struct Iter {
    inner: std::vec::IntoIter<usize>,
}

#[pyproto]
impl PyIterProtocol for Iter {
    fn __next__(mut slf: PyRefMut<'p, Self>) -> PyResult<Option<usize>> {
        Ok(slf.inner.next())
    }
}

#[pyclass]
struct Container {
    iter: Vec<usize>,
}

#[pyproto]
impl PyIterProtocol for Container {
    fn __iter__(slf: PyRef<'p, Self>) -> PyResult<Py<Iter>> {
        let iter = Iter {
            inner: slf.iter.clone().into_iter(),
        };
        PyCell::new(slf.py(), iter).map(Into::into)
    }
}

You can test this example by

    let gil = Python::acquire_gil();
    let py = gil.python();
    let inst = pyo3::PyCell::new(
        py,
        Container {
            iter: vec![1, 2, 3, 4],
        },
    )
    .unwrap();
    py_assert!(py, inst, "list(inst) == [1, 2, 3, 4]");

(please use # to hide the test).

In addition, it would be helpful to refer to Python doc.

[abingham]

Ah, I hadn't thought to check for default implementations of the protocol functions!

we don't need to implement next for containers.

The version provided by the trait just panics (it calls unimplemented!() which I think panics, at least), and this seems like a mistake. Callers in Python could, at least in principle, call __next__() on anything and expect their program to, at worst, throw an exception. So it seems like bad advice to tell people not to implement it.

Regarding your suggestion for the iterator implementation: iterators always need to implement __iter__(). It's a requirement of the Python iterator protocol, and it certainly shouldn't panic. It's called as a matter of course in certain Python code.

I may well be missing something important about the current implementation of PyIterProtocol, but it feels a bit unfinished. Let me know if I'm way off base, though.

[kngwyu]

The version provided by the trait just panics (it calls unimplemented!() which I think panics, at least), and this seems like a mistake.

No. It's a bit difficult to explain, but unimplemented is just a hacky implementation detail and never called.
If a user doesn't provide __next__, we just don't set tp_iternext for the class.
E.g., when we call next(inst) in the above example, we get:

TypeError: 'Container' object is not an iterator

So I don't want to recommend users to implement a stub for __next__.

Regarding your suggestion for the iterator implementation: iterators always need to implement iter().

Yeah, you're right. Please fix the example to do so.

[abingham]

It's a bit difficult to explain

I bet! I looked through that code a bit, but it's a bit dense :) I'll make the change you suggest regarding __next__. Thanks for taking the time to explain.

[abingham]

@kngwyu The tests are failing because py_assert! doesn't seem to be defined. Is it supposed to be made available somehow in the doc testing environment, or do I need to import it somehow?

[kngwyu]

@abingham
Ah, sorry we can use it only in the test files.
Please use pyo3::py_run!(py, inst, "assert ~") instead.

[kngwyu]

LGTM, thank you!

[abingham]

Thank you! I learned a lot.