Skip to content
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

Jump to bottom

bpo-42451: Indicate that PyTuple_GetItem does not support negative indices #23529

Merged
merged 7 commits into from
Nov 30, 2020
Merged

bpo-42451: Indicate that PyTuple_GetItem does not support negative indices #23529

merged 7 commits into from
Nov 30, 2020

Conversation

0xYasser
Copy link
Contributor

@0xYasser 0xYasser commented Nov 27, 2020 •
edited
Loading

@bedevere-bot bedevere-bot added docs Documentation in the Doc dir awaiting review labels Nov 27, 2020
@0xYasser 0xYasser marked this pull request as ready for review November 27, 2020 05:58
vstinner
vstinner reviewed Nov 27, 2020
View reviewed changes
Doc/c-api/tuple.rst
@@ -56,8 +56,10 @@ Tuple Objects

.. c:function:: PyObject* PyTuple_GetItem(PyObject *p, Py_ssize_t pos)

Return the object at position *pos* in the tuple pointed to by *p*. If *pos* is
out of bounds, return ``NULL`` and set an :exc:`IndexError` exception.
Copy link
Member

Choose a reason for hiding this comment

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

You can say:

If pos is negative or out of bound, ...

To document that IndexError is raised if pos is negative.

Misc/NEWS.d/next/Documentation/2020-11-26-21-45-09.bpo-42451.i4QoA_.rst Outdated
@@ -0,0 +1,2 @@
update PyTuple_GetItem docs to indicate that it does not support negative
Copy link
Member

Choose a reason for hiding this comment

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

I don't think that it's worth it, you can remove it ;-) I add the skip news label.

@terryjreedy
Copy link
Member

After I removed the trailing space on line 59, I noticed that it was not added by the PR but existed before. So a) the previous change to this doc must have been before we checked .rst files and b) there might be other trailing whitespace lines. If there are, and the whitespace tests fail again, please run python tools/scripts/patchcheck.py in the repository directory. (As described in the devguide somewhere.)

@terryjreedy
Copy link
Member

terryjreedy commented Nov 28, 2020 •
edited
Loading

Victor, do you otherwise consider this ready to merge? Want me to do it?
Update: previously failing tests now pass, so that mush have been the only culprit.

@0xYasser 0xYasser requested a review from vstinner November 30, 2020 04:49
vstinner
vstinner approved these changes Nov 30, 2020
View reviewed changes
Copy link
Member

@vstinner vstinner left a comment

Choose a reason for hiding this comment

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

LGTM.

@vstinner vstinner merged commit 9f00463 into python:master Nov 30, 2020
@vstinner
Copy link
Member

Merged, thanks!

adorilson pushed a commit to adorilson/cpython that referenced this pull request Mar 13, 2021
@0xYasser @adorilson
bpo-42451: Indicate that PyTuple_GetItem does not support negative in…
1d62494 
…dices (pythonGH-23529)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vstinner vstinner vstinner approved these changes

Assignees
No one assigned
Labels
docs Documentation in the Doc dir skip news
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@0xYasser @terryjreedy @vstinner @the-knights-who-say-ni @bedevere-bot