Accommodate Sphinx option by changing docstring return type of "integer" to "int". #100989

timoludwig · 2023-01-12T15:12:48Z

Bug report

When collections.deque is subclassed and the resulting class is documented with Sphinx using the option :inherited-members:, the following error appears:

WARNING: py:class reference target not found: integer -- return number of occurrences of value

I assume this is because the PyDoc_STRVAR docstring of the C-implementation is not compliant to PEP-7 as required in PyDoc_STRVAR:

cpython/Modules/_collectionsmodule.c

Lines 992 to 993 in d9dff4c

    
           PyDoc_STRVAR(count_doc, 
        
           "D.count(value) -> integer -- return number of occurrences of value");

And everything after -> is interpreted as type hint.

This can be reproduced with e.g. a python module sub_deque.py:

class SubDeque(deque):
    pass

and a documenation file docs/src/sub_deque.rst:

.. automodule:: sub_deque
   :inherited-members:

which is then built with Sphinx: sphinx-build -W docs/src docs/dist.

I'd be happy to provide a patch for this myself if you feel this issue should be fixed.

Your environment

CPython versions tested on: 3.7 - 3.11
Operating system and architecture: Arch Linux & Ubuntu

Linked PRs

gh-100989: Fix docstrings of collections.deque #100990

The text was updated successfully, but these errors were encountered:

rhettinger · 2023-01-12T21:56:43Z

You can change "integer" to "int" if you like. The other changes look gratuitous.

Also, PEP 7 predates modern type annotations. Doc strings are allowed to put any text as the return type as long as it is interpretable by a human. It is unfortunate that Sphinx is assuming otherwise. How would it handle user defined types or types defined in type comments?

timoludwig · 2023-01-12T22:36:00Z

Ok, thanks for your feedback!

You can change "integer" to "int" if you like. The other changes look gratuitous.

Still, changing it to D.count(value) -> int -- return number of occurrences of value would just cause Sphinx to literally search for the type int -- return number of occurrences of value which it wouldn't find either. Are you fine with including the new line after the return type? So D.count(value) -> int\n ...? Most of the other modules seem to follow that convention:

cpython/Modules/atexitmodule.c

Line 211 in 0ace820

"_ncallbacks() -> int\n\

cpython/Modules/_threadmodule.c

Line 190 in 762745a

"acquire(blocking=True, timeout=-1) -> bool\n\

cpython/Modules/nismodule.c

Line 28 in 81f7359

"get_default_domain() -> str\n\

Although to be fair, I also found quite a few other modules which use an inconsistent format, e.g.

cpython/Modules/timemodule.c

Line 160 in 3e06b50

"time() -> floating point number\n\

which I would change to time() -> float to match the correct type and allow the cross-referencing between documentations.

Also, PEP 7 predates modern type annotations.

Are there better options to annotate types in C modules? If so, I think it should be mentioned in the docs of PyDoc_STRVAR...

How would it handle user defined types or types defined in type comments?

User defined types are usually documented in the project that Sphinx is invoked in, and thus it'll find the reference to the custom type.

timoludwig added the type-bug An unexpected behavior, bug, or error label Jan 12, 2023

timoludwig added a commit to timoludwig/cpython that referenced this issue Jan 12, 2023

pythonGH-100989: Fix docstrings of collections.deque

425bfa4

timoludwig added a commit to timoludwig/cpython that referenced this issue Jan 12, 2023

pythongh-100989: Fix docstrings of collections.deque

6a66e8a

timoludwig mentioned this issue Jan 12, 2023

gh-100989: Fix docstrings of collections.deque #100990

Open

rhettinger changed the title ~~Docstrings of collections.deque not compliant to PEP-7~~ Accommodate Sphinx option by changing docstring return type of "integer" to "int". Jan 12, 2023

Accommodate Sphinx option by changing docstring return type of "integer" to "int". #100989

Accommodate Sphinx option by changing docstring return type of "integer" to "int". #100989

timoludwig commented Jan 12, 2023 •

edited by bedevere-bot

rhettinger commented Jan 12, 2023

timoludwig commented Jan 12, 2023

Accommodate Sphinx option by changing docstring return type of "integer" to "int". #100989

Accommodate Sphinx option by changing docstring return type of "integer" to "int". #100989

Comments

timoludwig commented Jan 12, 2023 • edited by bedevere-bot

Bug report

Your environment

Linked PRs

rhettinger commented Jan 12, 2023

timoludwig commented Jan 12, 2023

timoludwig commented Jan 12, 2023 •

edited by bedevere-bot