gh-131885: Updates docs to make max and min iterable param positional only

[ekohilas]

This PR updates the documentation of max and min to match their implementation like how sorted's implementation matches it's documentation.

Sorted's function definition in the documentation is as follows:

sorted(iterable, /, *, key=None, reverse=False)

When attempted to call with iterable as a keyword argument, the following occurs:

>>> sorted(iterable=[1])
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
TypeError: sorted expected 1 argument, got 0

However, looking at the documentation for max:

max(iterable, *, key=None)

And comparing to their implementation:

>>> max(iterable=[1])
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
TypeError: max expected at least 1 argument, got 0

The same follows for min

---

📚 Documentation preview 📚: https://cpython-previews--131868.org.readthedocs.build/

• Issue: Update function signatures to use * and / as needed #131885

[ekohilas]

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.

[skirpichev]

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

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

CC @AA-Turner

[skirpichev]

This is not a valid Python syntax. Same for max().

[ekohilas]

I understand the issue being that the square brackets make it invalid syntax, and that without this change, the documentation would still be wrong.

The reason for change here is to ensure that the web docs align with the code docs, similar to sorted:

cpython/Python/bltinmodule.c

Line 2575 in 5a3f13b

"sorted($module, iterable, /, *, key=None, reverse=False)\n"

Would it be more correct to instead write:

Suggested change

	"min(iterable, /, *[, default=obj, key=func]) -> value\n\
	"min(iterable, /, *, key=None) -> value\n\
	min(iterable, /, *, default, key=None) ->value\n\
	min(arg1, arg2, *args, /, *, key=None) ->value\n\

[ekohilas]

@skirpichev Could you please help me understand why -> value should be removed?

[skirpichev]

Rationale is just same as for sphinx docs: it's not a valid syntax (you will get a NameError trying to parse such signature with inspect.signature()).

[picnixz]

I understand the issue being that the square brackets make it invalid syntax, and that without this change, the documentation would still be wrong.

Actually, it's a valid Sphinx syntax. It's the "legacy" way of writing signatures. Many built-ins are written that way. As for ->, I think it was meant for "readability" even if it's unparsable. I would personally leave this untouched in this PR (namely, only add positional-only marker but leave out the ->). In a follow-up, we could clean-up the way "invalid" syntaxes, but this is something different than adding the markers.

[ekohilas]

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.

[ekohilas]

Created issue: #131885

[picnixz]

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.

[skirpichev]

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

[picnixz]

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

[picnixz]

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.

[AA-Turner]

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.

[picnixz]

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

[skirpichev]

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.

[picnixz]

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

[AA-Turner]

We should avoid mixing C-style [,optional-arg] with the / etc markers.

I think we should also not conflate changes to the documentation, which support a wider range of syntax, with changes to function signatures in docstrings, which are interpreted by pydoc, help(), etc. See Raymond's Discourse post on improving support for signatures, and Serhiy's draft work on multi-signature support for inspect.

For now, please revert the changes to bltinmodule.c, and we can start with iterative improvements to the rST documentation.

A

[bedevere-app]

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!

[skirpichev]

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

[ekohilas]

@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?

[skirpichev]

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

[terryjreedy]

In #137610, Serhiy proposes to put /s everywhere they belong, and I strongly support this. HIs PR #137609 includes the 2 additions here, so he also approves them. I think that this should be merged first so that OP gets the proper credit.

[terryjreedy]

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

[miss-islington-app]

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!

[bedevere-app]

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

[bedevere-app]

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