Skip to content

gh-129622: Clarify the set.is{sub,super}set docstrings #129637

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.

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 3 additions & 3 deletions Objects/clinic/setobject.c.h

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 4 additions & 4 deletions Objects/setobject.c
Original file line number Diff line number Diff line change
Expand Up @@ -2021,12 +2021,12 @@ set.issubset
other: object
/

Report whether another set contains this set.
Return True if the set is a subset of other.
[clinic start generated code]*/

static PyObject *
set_issubset_impl(PySetObject *so, PyObject *other)
/*[clinic end generated code: output=b2b59d5f314555ce input=f2a4fd0f2537758b]*/
/*[clinic end generated code: output=b2b59d5f314555ce input=0c956d86fff5b094]*/
{
setentry *entry;
Py_ssize_t pos = 0;
Expand Down Expand Up @@ -2066,12 +2066,12 @@ set.issuperset
other: object
/

Report whether this set contains another set.
Return True if the set is a superset of other.
Copy link
Member

Choose a reason for hiding this comment

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

IIUC, the OP issue is about return value. I think we can replace "report" with "Return True if", but why change the rest? Superset/subset - terminology, that might be unfamiliar to readers.

Copy link
Member

Choose a reason for hiding this comment

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

My specific suggestion was "Return True if self is a subset of other, else False.", with changing 'Report' with 'Return' the prime focus. I do not know if 'if else' is needed. Without looking around, I suspect both implicit and explicit alternatives are used and would be satified either way.

I used the parameter names (marked as italicized) as that is a general policy. For frozenset, the parameter names are used in all the dunder docstrings (but without *s) but never with the non-dunder functions. The look sloppy to me. The non-specific 'another set' seems weird to me compared to the specific 'other'. As for the superset relationship , the rewording of the repetitious 'is superset' as 'contains' (and similarly for 'is subset') is fine with me and even a good idea. My revised suggestion would then be "Return True if self contains other." and "Return True if other contains self." (both without or with ", else False")

Copy link
Member Author

@AA-Turner AA-Turner Feb 4, 2025

Choose a reason for hiding this comment

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

As far as I can tell, none of the docstrings in setobject.c use 'self'. Other docstrings in the module reference self as "a" / "this" / "the" set. Perhaps?:

Return True if this set contains other. and Return True if this set is contained by other.

My slight preference for {sub/super}set is as "contains" is ambiguous for the case of set equality, where both methods return True.

[clinic start generated code]*/

static PyObject *
set_issuperset_impl(PySetObject *so, PyObject *other)
/*[clinic end generated code: output=ecf00ce552c09461 input=5f2e1f262e6e4ccc]*/
/*[clinic end generated code: output=ecf00ce552c09461 input=c3b6ed4108639318]*/
{
if (PyAnySet_Check(other)) {
return set_issubset((PySetObject *)other, (PyObject *)so);
Expand Down
Loading