PyAnyMethods::iter() is a footgun, should be called try_iter()

[itamarst]

Bug Description

PyAnyMethods::iter() returns a PyResult<&Bound<PyIterator>>. And Result is an iterator. So it's very easy to accidentally iterate over it directly.

Steps to Reproduce

Consider the two seemingly identical functions:

fn iterate(s: Vec<&PyResult<Bound<PyAny>>>) {
    for obj in s.iter() {
        do_something(obj);
    }
}

fn iterate2(s: Bound<PySequence>) {
    for obj in s.iter() {
        do_something(obj);
    }
}

They seem identical, but iterate2() will iterate over the PyResult returned from PyAnyMethods::iter(), so it will always have a single item, the original sequence!

And yes, I just discovered this the hard way.

Your PyO3 version

0.22

Additional Info

I would suggest:

1. Deprecating iter().
2. Creating a new, identical function and calling it try_iter(). The new name will suggest that it returns a Result<> and therefore requires a ? or equivalent handler.

[davidhewitt]

I think this is a good idea. Either try_iter() or py_iter() seem ok to me. Would be open to hearing other maintainers' opinions on names.

[LilyFoote]

I prefer try_iter - it has the important hint that iteration is fallible and the user should expect to handle a Result. py_iter tells us we're working with a Python object, but we already know that from the type of s.

[Icxolu]

try_iter sounds good to me as well 👍