Skip to content

gh-110527: Improve PySet_Clear docs #110528

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
Merged
merged 3 commits into from
Oct 16, 2023
Merged

gh-110527: Improve PySet_Clear docs #110528

merged 3 commits into from
Oct 16, 2023

Conversation

sobolevn
Copy link
Member

@sobolevn sobolevn commented Oct 8, 2023 •
edited by github-actions bot
Loading

Mention that it can return -1 and raise SystemError:

cpython/Objects/setobject.c

Lines 2291 to 2298 in 7e30821

int
PySet_Clear(PyObject *set)
{
if (!PySet_Check(set)) {
PyErr_BadInternalCall();
return -1;
}
return set_clear_internal((PySetObject *)set);

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

@sobolevn sobolevn requested a review from vstinner October 8, 2023 20:30
@bedevere-app bedevere-app bot added docs Documentation in the Doc dir skip news awaiting review labels Oct 8, 2023
@bedevere-app bedevere-app bot mentioned this pull request Oct 8, 2023
Closed
vstinner
vstinner reviewed Oct 8, 2023
View reviewed changes
Doc/c-api/set.rst Outdated
Empty an existing set of all elements.
Empty an existing set of all elements. Return ``0`` on
success or ``-1`` on failure. Raise :exc:`SystemError` if *set* is not an
instance of :class:`set` or its subtype.
Copy link
Member

Choose a reason for hiding this comment

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

This doc is quite generic and doesn't explain that it cannot fail if you pass a set type or a subtype of the set type.

It only fails the argument type is not set.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done, should be better now!

vstinner
vstinner reviewed Oct 11, 2023
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.

@serhiy-storchaka: Do we have to document SystemError?

serhiy-storchaka
serhiy-storchaka reviewed Oct 11, 2023
View reviewed changes
Copy link
Member

@serhiy-storchaka serhiy-storchaka left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Other functions have:

Raise :exc:`SystemError` if *set* is not an instance of :class:`set` or its subtype.

I think the same wording should be used here, until there are good reasons for difference.

@vstinner
Copy link
Member

I think the same wording should be used here, until there are good reasons for difference.

I don't have a strong preference between subtype and subclass. Consistent is good.

vstinner
vstinner approved these changes Oct 12, 2023
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.

@serhiy-storchaka: I let you decide which phrasing is better for subtypes 😁

@vstinner vstinner merged commit bfc1cd8 into python:main Oct 16, 2023
@vstinner vstinner added needs backport to 3.11 only security fixes needs backport to 3.12 only security fixes labels Oct 16, 2023
@miss-islington
Copy link
Contributor

Thanks @sobolevn for the PR, and @vstinner for merging it 🌮🎉.. I'm working now to backport this PR to: 3.11.
🐍🍒⛏🤖

@miss-islington
Copy link
Contributor

Thanks @sobolevn for the PR, and @vstinner for merging it 🌮🎉.. I'm working now to backport this PR to: 3.12.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Oct 16, 2023
@sobolevn @miss-islington
pythongh-110527: Improve PySet_Clear docs (pythonGH-110528)
a68bce3 
(cherry picked from commit bfc1cd8)

Co-authored-by: Nikita Sobolev <[email protected]>
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Oct 16, 2023
@sobolevn @miss-islington
pythongh-110527: Improve PySet_Clear docs (pythonGH-110528)
04635d7 
(cherry picked from commit bfc1cd8)

Co-authored-by: Nikita Sobolev <[email protected]>
@bedevere-app
Copy link

bedevere-app bot commented Oct 16, 2023

GH-110927 is a backport of this pull request to the 3.11 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.11 only security fixes label Oct 16, 2023
@bedevere-app
Copy link

bedevere-app bot commented Oct 16, 2023

GH-110928 is a backport of this pull request to the 3.12 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.12 only security fixes label Oct 16, 2023
vstinner pushed a commit that referenced this pull request Oct 16, 2023
@miss-islington @sobolevn
[3.12] gh-110527: Improve PySet_Clear docs (GH-110528) (#110928)
b4ed730 
gh-110527: Improve `PySet_Clear` docs (GH-110528)
(cherry picked from commit bfc1cd8)

Co-authored-by: Nikita Sobolev <[email protected]>
vstinner pushed a commit that referenced this pull request Oct 16, 2023
@miss-islington @sobolevn
[3.11] gh-110527: Improve PySet_Clear docs (GH-110528) (#110927)
616862d 
gh-110527: Improve `PySet_Clear` docs (GH-110528)
(cherry picked from commit bfc1cd8)

Co-authored-by: Nikita Sobolev <[email protected]>
aisk pushed a commit to aisk/cpython that referenced this pull request Feb 11, 2024
@sobolevn @aisk
pythongh-110527: Improve PySet_Clear docs (python#110528)
d309a96
Glyphack pushed a commit to Glyphack/cpython that referenced this pull request Sep 2, 2024
@sobolevn @Glyphack
pythongh-110527: Improve PySet_Clear docs (python#110528)
0eaf39b
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

@serhiy-storchaka serhiy-storchaka serhiy-storchaka 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.
4 participants
@sobolevn @vstinner @miss-islington @serhiy-storchaka