8000 gh-101100: Fix sphinx warnings in `enum` module by sobolevn · Pull Request #101122 · python/cpython · GitHub
[go: up one dir, main page]
Skip to content

gh-101100: Fix sphinx warnings in enum module #101122

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

Merged
merged 4 commits into from
Jan 20, 2023
Merged

gh-101100: Fix sphinx warnings in enum module #101122

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
31d4b1e
gh-101100: Fix sphinx warnings in `enum` module
sobolevn Jan 18, 2023
52eb798
Address review
sobolevn Jan 19, 2023
440385f
Apply suggestions from code review
ethanfurman Jan 20, 2023
8f5716e
Update Doc/library/enum.rst
ethanfurman Jan 20, 2023
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
Next Next commit
gh-101100: Fix sphinx warnings in enum module
  • Loading branch information
@sobolevn
sobolevn committed Jan 18, 2023
commit 31d4b1ef7317e63d426744011b5b3ffd67a0dcd3
24 changes: 12 additions & 12 deletions Doc/library/enum.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,11 +52,11 @@ are not normal Python classes. See

.. note:: Nomenclature

- The class :class:`Color` is an *enumeration* (or *enum*)
- The attributes :attr:`Color.RED`, :attr:`Color.GREEN`, etc., are
- The class ``Color`` is an *enumeration* (or *enum*)
- The attributes ``Color.RED``, ``Color.GREEN``, etc., are
Copy link
Member
@CAM-Gerlach CAM-Gerlach Jan 18, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

If @ethanfurman prefers these to be verbatim code literals, as you've changed them to, instead of marked-up classes and attributes as they were before, then that's fine—it is a bit of a gray area.

However, in general when you're changing existing text, it seems to make more sense to at least initially propose the more minimal change that retains the previous markup, formatting and rendering, and just disables resolution (silencing the warnings):

Suggested change
- The class ``Color`` is an *enumeration* (or *enum*)
- The attributes ``Color.RED``, ``Color.GREEN``, etc., are
- The class :class:`!Color` is an *enumeration* (or *enum*)
- The attributes :attr:`!Color.RED`, :attr:`!Color.GREEN`, etc., are

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't quite agree that these references should be :class: and :attr:, because Color is only defined in the docs. But, visual separation is a good enough argument to keep it this way.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, I think there's a good conceptual argument to be made for either way, and I don't have a particular preference myself—in fact, if anything generally I tend to prefer the style you used, for the reason you mentioned (plus its a little simpler).

However, I ended up mentioning it here because it preserved the existing syntax and styling as much as practical, and in particular because this particular usage is explicitly to highlight the semantic differences between the enum, members, and their names and values, which the markup helps accentuate (in fact, before I saw the next line, I hesitated to make this comment at all).

To note, though, due to a theme CSS bug, references in admonitions (like this note) are rendered with the background anyway—but will start working again as soon as that's fixed.

*enumeration members* (or *members*) and are functionally constants.
- The enum members have *names* and *values* (the name of
:attr:`Color.RED` is ``RED``, the value of :attr:`Color.BLUE` is
``Color.RED`` is ``RED``, the value of ``Color.BLUE`` is
``3``, etc.)

---------------
Expand Down Expand Up @@ -165,8 +165,8 @@ Data Types
to subclass *EnumType* -- see :ref:`Subclassing EnumType <enumtype-examples>`
for details.

*EnumType* is responsible for setting the correct :meth:`__repr__`,
:meth:`__str__`, :meth:`__format__`, and :meth:`__reduce__` methods on the
*EnumType* is responsible for setting the correct *__repr__*,
*__str__*, *__format__*, and *__reduce__* methods on the
final *enum*, as well as creating the enum members, properly handling
duplicates, providing iteration over the enum class, etc.

Expand Down Expand Up @@ -424,9 +424,9 @@ Data Types
Using :class:`auto` with :class:`IntEnum` results in integers of increasing
value, starting with ``1``.

.. versionchanged:: 3.11 :meth:`__str__` is now :func:`int.__str__` to
.. versionchanged:: 3.11 *__str__* is now ``int.__str__`` to
Copy link
Member

Choose a reason for hiding this comment

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

Same here; why not:

Suggested change
.. versionchanged:: 3.11 *__str__* is now ``int.__str__`` to
.. versionchanged:: 3.11 :meth:`~object.__str__` is now :meth:`!int.__str__` to

Copy link
Member Author

Choose a reason for hiding this comment

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

I have slightly modified your proposed change, because reading

:meth:`~object.__str__` is now :meth:`!int.__str__`

does not make much sense to me. Now it is:

:meth:`!__str__` is now :meth:`!int.__str__`

Copy link
Member

Choose a reason for hiding this comment

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

But the latter is essentially what readers will actually see, of course; the ~ removes the object prefix, so the original suggestion already displays as your second example, just with __str__ usefully cross-referenced to a description of the __str__ dunder and what it's used for, which seems to me to be potentially helpful to readers (and surely does no harm, at least).

better support the *replacement of existing constants* use-case.
:meth:`__format__` was already :func:`int.__format__` for that same reason.
*__format__* was already ``int.__format__`` for that same reason.


.. class:: StrEnum
Expand Down Expand Up @@ -761,11 +761,11 @@ Data Types
Supported ``__dunder__`` names
""""""""""""""""""""""""""""""

:attr:`__members__` is a read-only ordered mapping of ``member_name``:``member``
:attr:`!EnumType.__members__` is a read-only ordered mapping of ``member_name``:``member``
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 really get this either. This is a list of supported dunder and sunder names, and none of the other names have the class name explicitly prefixed, so I'm not sure why it makes sense to treat this differently.

Either __members__ should be explicitly documented under EnumType, like a number of other dunders and sunders are (including some listed here), and the link fixed here in the meantime (which displays the same as now until it is added):

Suggested change
:attr:`!EnumType.__members__` is a read-only ordered mapping of ``member_name``:``member``
:attr:`~EnumType.__members__` is a read-only ordered mapping of ``member_name``:``member``

or, if for whatever reason it doesn't make sense to explicitly document __members__ in the usual way, silence the warning without the prefix:

Suggested change
:attr:`!EnumType.__members__` is a read-only ordered mapping of ``member_name``:``member``
:attr:`!__members__` is a read-only ordered mapping of ``member_name``:``member``

Copy link
Member Author

Choose a reason for hiding this comment

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

Let's document __member__ separately and keep the focus of this PR focused 😉

Copy link
Member

Choose a reason for hiding this comment

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

Sure, but if __members__ should be documented (or the decision left until later), then deresolving it not only silences a valid warning, but also means that it will stay broken even if and when __members__ is documented, rather than start working automatically.

Therefore, unless @ethanfurman has some reason to not explicitly document __members__ but still list it here (which is possible), then the default approach should be the former, i.e. to fix it but leave it resolving, unless and until it is decided to explicitly leave it undocumented.

items. It is only available on the class.

:meth:`__new__`, if specified, must create and return the enum members; it is
also a very good idea to set the member's :attr:`_value_` appropriately. Once
``__new__``, if specified, must create and return the enum members; it is
also a very good idea to set the member's ``_value_`` appropriately. Once
all the members are created it is no longer used.


Expand Down Expand Up @@ -804,7 +804,7 @@ Utilities and Decorators
.. class:: auto

*auto* can be used in place of a value. If used, the *Enum* machinery will
call an *Enum*'s :meth:`_generate_next_value_` to get an appropriate value.
call an *Enum*'s :meth:`!Enum._generate_next_value_` to get an appropriate value.
For *Enum* and *IntEnum* that appropriate value will be the last value plus
one; for *Flag* and *IntFlag* it will be the first power-of-two greater
than the highest value; for *StrEnum* it will be the lower-cased version of
Expand Down Expand Up @@ -847,7 +847,7 @@ Utilities and Decorators
.. decorator:: unique

A :keyword:`class` decorator specifically for enumerations. It searches an
enumeration's :attr:`__members__`, gathering any aliases it finds; if any are
enumeration's :attr:`!EnumType.__members__`, gathering any aliases it finds; if any are
found :exc:`ValueError` is raised with the details::

>>> from enum import Enum, unique
Expand Down
0