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

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

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!

Co-authored-by: Evan Kohilas <ekohilas@users.noreply.github.com>

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!

…)`` (pythonGH-131868) (cherry picked from commit dd079db) Co-authored-by: Evan Kohilas <ekohilas@users.noreply.github.com>

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.

…()`` (GH-131868) (#137657) Co-authored-by: Evan Kohilas <ekohilas@users.noreply.github.com>

* main: pythongh-137288: Update 3.14 magic numbers (pythonGH-137665) pythongh-135228: When @DataClass(slots=True) replaces a dataclass, make the original class collectible (take 2) (pythonGH-137047) pythongh-126008: Improve docstrings for Tkinter cget and configure methods (pythonGH-133303) pythongh-131885: Use positional-only markers for ``max()`` and ``min()`` (python#131868) pythonGH-137426: Remove code deprecation of `importlib.abc.ResourceLoader` (pythonGH-137567) pythongh-125897: Mark range function parameters as positional only (python#125945) pythongh-137400: Fix a crash when disabling profiling across all threads (pythongh-137471) pythongh-115766: Fix IPv4Interface.is_unspecified (pythonGH-137326) pythongh-128813: cleanup C-API docs for PyComplexObject (pythonGH-137579) pythongh-135953: Profile a module or script with sampling profiler (python#136777) Fix documentation of hash in PyHash_FuncDef (python#137595)

…)`` (python#131868)

…()`` (GH-131868) (#137656) gh-131885: Use positional-only markers for ``max()`` and ``min()`` (GH-131868) (cherry picked from commit dd079db) Co-authored-by: Evan Kohilas <ekohilas@users.noreply.github.com>

bedevere-app Bot added docs Documentation in the Doc dir skip news labels Mar 29, 2025

github-project-automation Bot added this to Docs PRs Mar 29, 2025

github-project-automation Bot moved this to Todo in Docs PRs Mar 29, 2025

ekohilas marked this pull request as ready for review March 29, 2025 05:26

ekohilas requested a review from ericsnowcurrently as a code owner March 29, 2025 05:26

bedevere-app Bot added the awaiting review label Mar 29, 2025

skirpichev reviewed Mar 29, 2025

View reviewed changes

ekohilas commented Mar 30, 2025

View reviewed changes

skirpichev reviewed Mar 30, 2025

View reviewed changes

skirpichev changed the title ~~Updates docs to make max and min iterable param positional only~~ Mar 30, 2025

bedevere-app Bot mentioned this pull request Mar 30, 2025

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

Open

5 tasks

picnixz reviewed Mar 30, 2025

View reviewed changes

AA-Turner requested changes Mar 30, 2025

View reviewed changes

bedevere-app Bot removed the awaiting review label Mar 30, 2025

bedevere-app Bot added the awaiting changes label Mar 30, 2025

AA-Turner removed the request for review from ericsnowcurrently March 30, 2025 17:19

ekohilas and others added 4 commits April 12, 2025 10:51

Update functions.rst

c48ff1b

Update bltinmodule.c for min and max

df21826

Apply suggestions from code review …

e3399a2

Co-authored-by: Evan Kohilas <ekohilas@users.noreply.github.com>

revert

2e26c23

ekohilas force-pushed the patch-1 branch from 8771da9 to 2e26c23 Compare April 12, 2025 00:51

skirpichev mentioned this pull request Aug 10, 2025

gh-137609: Update signatures of builtins in the documentation #137610

Merged

terryjreedy approved these changes Aug 11, 2025

View reviewed changes

bedevere-app Bot added awaiting merge and removed awaiting changes labels Aug 11, 2025

AA-Turner added needs backport to 3.13 needs backport to 3.14 bugs and security fixes labels Aug 11, 2025

AA-Turner approved these changes Aug 11, 2025

View reviewed changes

bedevere-app Bot removed the awaiting merge label Aug 11, 2025

github-project-automation Bot moved this from Todo to Done in Docs PRs Aug 11, 2025

bedevere-app Bot removed the needs backport to 3.14 bugs and security fixes label Aug 11, 2025

bedevere-app Bot removed the needs backport to 3.13 label Aug 11, 2025

AA-Turner pushed a commit that referenced this pull request Aug 11, 2025

[3.13] gh-131885: Use positional-only markers for max() and ``min… …

c1e1c88

…()`` (GH-131868) (#137657) Co-authored-by: Evan Kohilas <ekohilas@users.noreply.github.com>

Agent-Hellboy pushed a commit to Agent-Hellboy/cpython that referenced this pull request Aug 19, 2025

pythongh-131885: Use positional-only markers for max() and ``min(… …

7da761a

…)`` (python#131868)

Conversation

ekohilas commented Mar 29, 2025 • edited by bedevere-app Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ekohilas commented Mar 29, 2025

Uh oh!

skirpichev left a comment

Choose a reason for hiding this comment

Uh oh!

ekohilas commented Mar 30, 2025

Uh oh!

ekohilas commented Mar 30, 2025

Uh oh!

picnixz commented Mar 30, 2025

Uh oh!

skirpichev commented Mar 30, 2025

Uh oh!

picnixz commented Mar 30, 2025

Uh oh!

picnixz commented Mar 30, 2025

Uh oh!

AA-Turner commented Mar 30, 2025

Uh oh!

picnixz commented Mar 30, 2025

Uh oh!

skirpichev commented Mar 30, 2025

Uh oh!

picnixz commented Mar 30, 2025

Uh oh!

AA-Turner left a comment

Choose a reason for hiding this comment

Uh oh!

bedevere-app Bot commented Mar 30, 2025

Uh oh!

skirpichev commented Apr 12, 2025

Uh oh!

ekohilas commented Apr 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

skirpichev commented Apr 12, 2025

Uh oh!

terryjreedy left a comment

Choose a reason for hiding this comment

Uh oh!

terryjreedy commented Aug 11, 2025

Uh oh!

Uh oh!

miss-islington-app Bot commented Aug 11, 2025

Uh oh!

bedevere-app Bot commented Aug 11, 2025

Uh oh!

bedevere-app Bot commented Aug 11, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

ekohilas commented Mar 29, 2025 •

edited by bedevere-app Bot

Loading

ekohilas commented Apr 12, 2025 •

edited

Loading