Update recipes.iter_index to match CPython PR 102360

[bbayles]

This PR updates more_itertools.recipes.iter_index to match python/cpython#102360.

We still support Python 3.7, so we're not using the Walrus operator.

[vbrozik]

The function iter_index(iterable, value, start=0) will silently suppress ValueError exceptions raised by a generator passed to the argument iterable.

[bbayles]

Is this also a problem with the version in the CPython docs?

[vbrozik]

Yes, thanks, it is the same. Just to be sure I prepared a demonstration of the bug here:

https://github.com/vbrozik/python-ntb/blob/main/problems_from_forums/2023-07-24_iter_index.ipynb

def assert_no_value(iterable, forbidden_value):
    """Pass the iterable but raise ValueError if forbidden_value is found."""
    for item in iterable:
        if item == forbidden_value:
            raise ValueError(f'Value {forbidden_value!r} is not allowed.')
        yield item

# Here we should get ValueError exception
# but it is being silently suppressed by iter_index()
list(iter_index(assert_no_value('AABCADEAF', 'B'), 'A'))

So I should probably create an issue for the CPython docs, right?

[bbayles]

Yes, probably a good idea!

[rhettinger]

@bbayles It's going to take a while for the CPython docs issue to resolve because the best solution involves adding a new exception type to distinguish between the two sources of ValueError. Without that change, operator.indexOf cannot be reliably used with iterators or exotic sequences.

For more-itertools, the safest play is to revert this PR which was only an optimization.

Alternatively, the optimization could be preserved by inspecting the exception to distinguish between the two cases:

try:                                                   # <==== new
    operator.indexOf('abc', 'd')                       # <==== new
except ValueError as e:                                # <==== new
    _index_of_value_error_repr = repr(e)               # <==== new
    
def iter_index(iterable, value, start=0):
    "Return indices where a value occurs in a sequence or iterable."
    # iter_index('AABCADEAF', 'A') --> 0 1 4 7
    try:
        seq_index = iterable.index
    except AttributeError:
        # Slow path for general iterables
        it = islice(iterable, start, None)
        i = start - 1
        try:
            while True:
                yield (i := i + operator.indexOf(it, value) + 1)
        except ValueError as e:
            if repr(e) != _index_of_value_error_repr:  # <==== new
                raise                                  # <==== new
    else:
        # Fast path for sequences
        i = start - 1
        try:
            while True:
                yield (i := seq_index(value, i+1))
        except ValueError:
            pass

[bbayles]

@rhettinger - thanks for the comments.

The code prior to the optimization in python/cpython#102360 also has a ValueError catch in the "Fast path for sequences," yes? I presume that one is also problematic, at least in theory.

[vbrozik]

@bbayles you are right. In the fast path the problem was already there although it is less common to define own sequence class raising a ValueError.

Here Stefan @pochmann replicated the problem: python/cpython#107208 (comment)

[bbayles]

Thanks - I subscribed to that thread.

[rhettinger]

I'm not concerned about ValueError capture for the sequence path. The technique of looping over seq.index() calls and catching ValueError is as old as the hills and is the only reason that index was ever given a start argument. The technique doesn't seem to have ever been a problem in practice and is sure not worth hobbling the recipe. That would just make everyone pay for a problem that no one actually has.