Skip to content

Conversation

@ghost
Copy link

@ghost ghost commented Mar 4, 2025

Divided the class description into a part about sets and a part about frozensets. Also, cleaned up the explanation of the comparison method.

Fixed gh-113746.


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

Divided the class description into a part about sets and a part about frozensets.
Also, cleaned up the explanation of the comparison method.

Fixed gh-113746.
Copy link
Member

@picnixz picnixz left a comment

Choose a reason for hiding this comment

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

I'm not convinced by this. Isn't there a way to not generate a link for frozenset.update actually? (we can probably hotfix it with a custom extension though)

cc @AA-Turner @rhettinger

Comment on lines 4486 to 4489
.. method:: set == other

Test whether every element of each set is contained in the other, that is,
``set <= other and set >= other``.
Copy link
Member

Choose a reason for hiding this comment

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

All objects implement == so it's redundant here IMO. In addition, we explain the differences with set == frozenset as well.

Comment on lines 4540 to 4545
Both :class:`set` and :class:`frozenset` support set to set comparisons. Two
sets are equal if and only if every element of each set is contained in the
other (each is a subset of the other). A set is less than another set if and
only if the first set is a proper subset of the second set (is a subset, but
is not equal). A set is greater than another set if and only if the first set
is a proper superset of the second set (is a superset, but is not equal).
Copy link
Member

Choose a reason for hiding this comment

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

Please keep this one.

x not in s
isdisjoint(other)
issubset(other)
set == other
Copy link
Member

Choose a reason for hiding this comment

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

Remove this one.


Instances of :class:`frozenset` provide the following :class:`set` operations:

.. method:: len(s)
Copy link
Member

Choose a reason for hiding this comment

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

This would render quite ugly IMO. If users use a link to frozenset.isdisjoint in their Sphinx project, then it will go there and then the user wouldn't have any proper description.

@rhettinger
Copy link
Contributor

I'm not convinced by this. Isn't there a way to not generate a link for frozenset.update actually? (we can probably hotfix it with a custom extension though)

I agree. The actual docs are much better as they are now. There is no redundancy and there docs are very clear about that operations are available for sets that do not apply to frozensets.

If anything needs to be "fixed" it is the indexing. The docs themselves are fine.

Marking this as closed. The solution to #113746 needs to be targeted at indexing rather than this indirect solution that makes the long stable documentation worse.

Thanks for the PR but I would rather live with the minor indexing buglet (or have it fixed in some other way) than to damage the set/frozenset documentation which has served users well for two decades.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting review docs Documentation in the Doc dir skip news

Projects

Status: Todo

Development

Successfully merging this pull request may close these issues.

set/frozenset methods intermixed in search, wrong results and target page anchor

2 participants