GeezerVison

Actually, I'm finding more examples of these cases such as map and range. If the above looks good I'll add more commits to include those too.

Actually, I'm finding more examples of these cases such as map and range. If the above looks good I'll add more commits to include those too.

Previously similar change was reverted: #99476 See also #125945

Thanks for these references!

While devguide suggests now using correct function signatures, I'm not sure we shold push this that way.

We definitely need an issue, e.g. to adjust all signatures on the builtin functions page. We should also consider if we could actually simplify function signatures to avoid '/' and/or '*' syntax. See https://discuss.python.org/t/81003

Thank you for linking this read!

I too would like to see simplifying these built-in function signatures, and have functions like range allow for keyword arguments.

As I understand it, doing so would be a one day door that would require additional thought and decisions, as if it went through and started to be used, it would no longer be backwards compatible.

Until then, this seems like the next best step. It's what the Editorial board has agreed on, it brings consistency, and reverting these changes in the future if and when they are simplified would be easy.

As it stands, without these changes:

• The documentation is inconsistent and causes confusion. e.g.
  • For max, * is used to highlight how the function works, but / isn't, even though it behaves like it is.
  • There is no reference in the docs to specify that max doesn't allow some arguments as keyword.
• It's difficult for users to understand what the correct usage is, or how to re-create these functions themselves. If a user copies this function definition to do so, it would not match the real definition.

Created issue: #131885

While devguide suggests now using correct function signatures, I'm not sure we shold push this that way.

Personally, I read this is as requirement and not a suggestion. Namely,

If a function accepts positional-only or keyword-only arguments, include the slash and the star in the signature as appropriate:

Here, the include is an imperative form and not a recommendation IMO. In addition, the rationale is (emphasis mine)

Although the syntax is terse, it is precise about the allowable ways to call the function and is taken from Python itself.

So I think it's fine to accept those changes. The reversal of previous changes was mostly done because it was incorrect, albeit eval() could have used a positional-only at the end.

So I think it's fine to accept those changes.

Probably, yes. But rather than mechanically accept "complex" signatures - we should revisit on case-by-case basis if we could simplify them. E.g. support bool(x) instead of bool(x, /).

The reversal of previous changes was mostly done because it was incorrect

@picnixz, are you about #99476? Reversion was argued not by correctness: #98340

Wait, were you talking about #100547 or another reversal?

Ah no I see. Ok you were talking about another PR. My bad. Well, the EB decision is quite recent and the reversal was done before that decision so I think it doesn't apply here.

Probably, yes. But rather than mechanically accept "complex" signatures - we should revisit on case-by-case basis if we could simplify them. E.g. support bool(x) instead of bool(x, /).

For this reason, I'm not sure the tracking issue is helpful -- it will encourage people to create many PRs to 'fix' the issue, when it is not just a mechanical transformation at hand.

I'm not sure the tracking issue is helpful

This is the reason why I didn't add the "easy" label. But tracking is fine I think. I can add a warning note to say that it's not an easy one as the transformation is not just mechanical

I'm not sure the tracking issue is helpful -- it will encourage people to create many PRs to 'fix' the issue, when it is not just a mechanical transformation at hand.

Someone could comment on issue to clarify it further.

I've added a red caution box, hopefully it will be seen

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

And if you don't make the requested changes, you will be put in the comfy chair!

@ekohilas, please avoid force-pushing to the PR.

@ekohilas, please avoid force-pushing to the PR.

Ah sorry I'm clicking the rebase button via GitHub.

Are there guidelines on why I should avoid force pushing in the cpython repo?

Sure, they listed in the devguide: https://devguide.python.org/getting-started/pull-request-lifecycle/#id2

@AA-Turner You requested removal of changes from the .c module, and this was done. Anything else? or dismiss change?

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

GH-137656 is a backport of this pull request to the 3.14 branch.

GH-137657 is a backport of this pull request to the 3.13 branch.