Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
AA-Turner merged 6 commits into from
Aug 11, 2025
Merged

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

Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
1fe4bd5
Updates functions.rst to make iterable positional only
ekohilas Mar 29, 2025
bd686b0
functions.rst for min
ekohilas Mar 29, 2025
c48ff1b
Update functions.rst
ekohilas Mar 29, 2025
df21826
Update bltinmodule.c for min and max
ekohilas Mar 29, 2025
e3399a2
Apply suggestions from code review
skirpichev Mar 30, 2025
2e26c23
revert
ekohilas Apr 11, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Update bltinmodule.c for min and max
  • Loading branch information
@ekohilas
ekohilas committed Apr 12, 2025
commit df21826c08eef70703e79fc5aabb38514fd17d64
4 changes: 2 additions & 2 deletions Python/bltinmodule.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2019,7 +2019,7 @@ builtin_min(PyObject *self, PyObject *const *args, Py_ssize_t nargs, PyObject *k
}

PyDoc_STRVAR(min_doc,
"min(iterable, *[, default=obj, key=func]) -> value\n\
"min(iterable, /, *[, default=obj, key=func]) -> value\n\
Copy link
Copy Markdown
Member

@skirpichev skirpichev Mar 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Copy Markdown
Contributor Author

@ekohilas ekohilas Mar 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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\

Copy link
Copy Markdown
Contributor Author

@ekohilas ekohilas Mar 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Copy Markdown
Member

@skirpichev skirpichev Mar 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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()).

Copy link
Copy Markdown
Member

@picnixz picnixz Mar 30, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread
ekohilas marked this conversation as resolved.
Outdated
min(arg1, arg2, *args, *[, key=func]) -> value\n\
Comment thread
ekohilas marked this conversation as resolved.
\n\
With a single iterable argument, return its smallest item. The\n\
Expand All @@ -2036,7 +2036,7 @@ builtin_max(PyObject *self, PyObject *const *args, Py_ssize_t nargs, PyObject *k
}

PyDoc_STRVAR(max_doc,
"max(iterable, *[, default=obj, key=func]) -> value\n\
"max(iterable, /, *[, default=obj, key=func]) -> value\n\
max(arg1, arg2, *args, *[, key=func]) -> value\n\
Comment thread
ekohilas marked this conversation as resolved.
\n\
With a single iterable argument, return its biggest item. The\n\
Expand Down