👍 Will check it out and update the PR

@ZeroIntensity I can seePyIter_NextItem being used only in couple places. Should I rename the issue to indicate migration from oldPyIter_Next to newPyIter_NextItem api and work on replacing old one in the entire codebase?

I don't think we need to replacePyIter_Next throughout theentire codebase (though it is probably inevitable), just change it for the cases that are problematic.

Should I rename the issue to indicate migration from old PyIter_Next

Nah, just update the PR title.

@ZeroIntensity so if this is inevitable (otherwise why would newPyIter_NextItem be implemented) and I can see ~63 instances ofPyIter_Next usage in the codebase, I would like to spend time on migrating to the new version in this PR. Or at least those instances that do not require significant rewrite and don't introduce more complexity. Any objections from your side? Or if this is still too much for a single PR, maybe create top level issue to track migration and add separate issues for migration in each file where usages occur?

If older Python versions have the same code, it will be needed to backport these changes. In such case it is better to use the older API, unless you want to add a work for yourself and for reviewers.

Ah, right. I would be in favor of a follow-up PR to switch toPyIter_NextItem though.

🤖 New build scheduled with the buildbot fleet by@picnixz for commit3aa46e7 🤖

If you want to schedule another build, you need to add the🔨 test-with-refleak-buildbots label again.

Regarding migration toPyIter_NextItem, can I create top level issue and tag you guys?@serhiy-storchaka@picnixz@ZeroIntensity ?

Yes, you can just request my review. But before creating the issue, you should locally verify that:

• the changes would not impact performances (I don't know which one is faster); try to check ifPyIter_Next is actually faster overall or if it'sPyIter_NextItem).
• some parts of the code directly usetp_iternext under some assumptions and for performance reasons. Those parts should not be modified.
• locally investigate how you want to split the task. Changing a lot of places may or may not be easy to review. There are more than 50 usages my IDE found so maybe splitting it up by file would be better.
• let's first focus on parts that don't need much rewrite; it's fine if some parts are still using the old API though.

FTR, changing the usages could perhaps make future backports harder, so I don't know whether it's worth the shot.

locally investigate how you want to split the task. Changing a lot of places may or may not be easy to review. There are more than 50 usages my IDE found so maybe splitting it up by file would be better.

Yes, my initial thought was to create top level issue to track all sub issues for each file separately to make review process easier.

FTR, changing the usages could perhaps make future backports harder, so I don't know whether it's worth the shot.

I presume this is not first time of migrating to newer API. How were those cases handled?

One more question. When comparing code forPyIter_Next:

Yes it's intentional. Docs (https://docs.python.org/3/c-api/iter.html#c.PyIter_Next) say:

The object must be an iterator according toPyIter_Check() (it is up to the caller to check this).

The reason is to make the call more efficient when the caller knows that the object is a correct iterator. OTOHPyIter_NextItem makes the check for the caller (ifPy_TYPE is a function call, this would add one function call and one pointer comparison; on incorrect inputs, we don't care about performances).

I don't know how less (or more?) efficient it would be but it might not necessarily by much. If you're really interested in preserving performance, we could first glance at the assembly and benchmarks some simple statements, though I suspect that naive benchmarks would hide too much noise (but maybe not?).

Now, if the caller is checking the input toPyIter_Next, then you can maybe remove that check and directly usePyIter_NextItem instead (in the end, you'll move the cost of the check toPyIter_NextItem, though it will cost a function call (I assume that the checks are actually inlined checks but maybe not)).

I presume this is not first time of migrating to newer API. How were those cases handled?

I don't know, I only became active with CPython in June :')

I presume this is not first time of migrating to newer API. How were those cases handled?

We migrate to newer API if it is faster, safer or more convenient. Or if there are plans to remove the older API.

There are no plans to removePyIter_Next(), it is not broken. In theory, it may be marginally faster because it does not need to callPyErr_Occurred(), but on other hand, additional checkPy_TYPE(iter)->tp_iternext == NULL can make it slower.

I would recommend to use a newer API in a new code if it makes the code clearer, and do not touch existing code (unless you have other reasons to rewrite it).

Thanks@WolframAlph for the PR, and@serhiy-storchaka for merging it 🌮🎉.. I'm working now to backport this PR to: 3.13.
🐍🍒⛏🤖 I'm not a witch! I'm not a witch!

Thanks@WolframAlph for the PR, and@serhiy-storchaka for merging it 🌮🎉.. I'm working now to backport this PR to: 3.12.
🐍🍒⛏🤖

Sorry,@WolframAlph and@serhiy-storchaka, I could not cleanly backport this to3.13 due to a conflict.
Please backport usingcherry_picker on command line.

cherry_picker 5c814c83cdd3dc42bd9682106ffb7ade7ce6b5b3 3.13

Sorry,@WolframAlph and@serhiy-storchaka, I could not cleanly backport this to3.12 due to a conflict.
Please backport usingcherry_picker on command line.

cherry_picker 5c814c83cdd3dc42bd9682106ffb7ade7ce6b5b3 3.12

GH-128272 is a backport of this pull request to the3.13 branch.

GH-128273 is a backport of this pull request to the3.12 branch.