Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork32k
gh-129622: Clarify theset.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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:main
Are you sure you want to change the base?
Conversation
@@ -2066,12 +2066,12 @@ set.issuperset | |||
other: object | |||
/ | |||
Report whether thissetcontains another set. | |||
Return True if thesetis a superset of other. |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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 ifself is a subset ofother, 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 ifself containsother." and "Return True ifother containsself." (both without or with ", else False")
There was a problem hiding this comment.
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 insetobject.c
use 'self'. Other docstrings in the module referenceself as "a" / "this" / "the" set. Perhaps?:
Return True if this set contains other.
andReturn 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 returnTrue
.
terryjreedy commentedFeb 4, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Test failure (Check if generated files are up to date) is build failure of |
Thoughts:
|
Uh oh!
There was an error while loading.Please reload this page.