CI failure due to S390x, not related to this PR.

OK, I did not get to new tests today, but this now adds only a single function NpyIter_FinalizeBuffers(iterator, error_state), some some wonkier logic in places where the API is technically not held (the iterator reset functions).

This fixes the following for example:

def leak():
    arr = np.arange(10000)
    for i in range(1000):
        it = np.nditer(arr.astype(np.int32), op_dtypes=[np.object_], flags=["buffered", "external_loop"])
        next(it)

this is a no-error example, examples with error should behave identically.

Alternatively shows the same issue with structured:

def leak():
    arr = np.arange(10000)
    for i in range(1000):
        it = np.nditer(arr.astype("i,i"), op_dtypes=[np.dtype("O,i")], flags=["buffered", "external_loop"])
        next(it)

All of these examples leak the numbers from 1 to 8192 (or so), since that is the size of the first buffer chunk.

Not adding the new dealloc function basically means that we call PyErr_Occurred() a few times for no particularly good reason at all, other than that it is fine. I think the new function itself is good, I am not quite sure about the return value, since I dislike NPY_SUCCEED, but its what NpyIter uses throughout :(.

sorry for the back and forth. I don't particularly like that the NpyIter user (and we) have to call `PyErr_Occurred(), but here is the solution relying on no new API, but rather on that.

In particular, I think this means that there is no way to even find out if a casting error occurred without PyErr_Occurred() (and thus, the GIL). Usually that should not matter of course, since the user will iterate exactly once (ufunc.at is different, but its terrible.)

Now, this is reasonable, as long as we are happy with leaving the error set (users can stash it), and the fact that users must use PyErr_Occurred() (or its implicit call in NpyIter_Dealloc().

I guess practically, it changes almost nothing except that iteration may get aborted, when before that was not possible.

OK, a bit of proactive thinking would help me here. We don't have to figure out the full picture yet, but we do have two issues:

1. There are a few places, which technically should float the error (the Reset* functions), but don't. Technically, these are called without the GIL, so it would possibly be nice to know if an error occurred without re-grabbing the GIL. I admit, that is super corner case API.

2. Technically, we actually have to check floating point error flags as well (see bullet point 1. below). I honestly think the only way to do this is to add a single new function, if I must privately for now. Which checks floating point flags as well as whether iteration was aborted due to a cast error (bullet point 2. below). This can be retrofitted to only require (and grab) the GIL when an error occurred, otherwise an error should already be set.

So who is supposed to check for errors? Especially for floating point errors? Either the iterator does it, or the iterator may need to be able to signal that FPEs should be checked, does that require more API than the existing Ufunc API?

And do we live with the fact that there may be no way to know if an error occurred until calling NpyIter_Dealloc? Its probably fine, but dunno...

The full picture:

1. We have currently broken behaviour, which also proofs that in the correct world we sometimes can only report errors after the fact:

    np.positive(np.array([3e300]*10000), dtype=np.float32)  # OverflowWarning, because ufuncs always check
    np.array([3e300]*10000).astype(dtype=np.float32)  # No warning, because NpyIter does not check
   

   I.e. the iterator may have to check for floating point error flags to behave correctly.

   • It is tempting to say that users should be allowed to do similar "after the fact" error checking. That seems like a can of worms to me, so I am tempted to disregard it. Users would have to grab the GIL and abort from the inner-loop. I.e. if we add cleanup, errors will still not be possible during cleanup! Only floating point errors are special (or any other new builtin error support).

2. When we have the API (the GIL cannot be released), a Python error can be set, and we should abort iteration immediately. We currently never do that (we use faith-based error handling after all). The only way the user (and NpyIter itself, but that could be fixed) can find about this currently is PyErr_Occurred()

   • We do sometimes (and should generally) set errors in random ufuncs, by temporarily grepping the GIL (maybe only integer powers), and especially for warnings that is (or will be) useful, IMO. It seems just a lot more convenient to handle errors right away normally. (Yes, this requires the PyGILState API, and we may need to reserve passing in more info at some point, but that seems viable) makes just as much sense for casting. Honestly, the only argument against this is floating point flags (where it would be costly) and some strange sense that ufunc inner-loops should work without Python, which in my opinion would be misguided.

OK, marking ready for review. Its messed up, but there is nothing to do about it. I think the tests should cover things fairly well.

I would be happy to think about what new API can help to fix the issue, and we may very much have to soon enough, but there is no obvious choice, so I can't just make a call on my own. And if we are unwilling to add new API, this is probably about as good as it can get.

The NpyIter changes should probably be looked at carefully, to ensure I did not accidentally modify things seriously. Generally, it seems the whole API might be a bit dubious about partial iterations, I guess its plausible nobody ever used that...

I don't think the codecov failure has too much relevance. Quite a few things simply can't be changed, or are obvious error-path corrections.

Thanks @seberg