sphinx-doc / sphinx

The Sphinx documentation generator
https://www.sphinx-doc.org/
Other
6.53k stars 2.12k forks source link

Resolve directives before running napoleon #9939

Open Bibo-Joshi opened 2 years ago

Bibo-Joshi commented 2 years ago

Is your feature request related to a problem? Please describe. At python-telegram-bot, we have many methods that require the same types of arguments, which should also get the same docstring. E.g. search for "chat_id (int | str) – Unique identifier for" at https://python-telegram-bot.readthedocs.io/en/stable/telegram.bot.html.

It would be helpful if one could write all those parameter specifications at a central place and import them as needed. This works when not relying on napoleon - e.g.

    .. include:: params.rst
    :param field_storage: The :class:`FileStorage` instance to wrap
    :type field_storage: FileStorage
    :param temporary: Whether or not to delete the file when the File
       instance is destructed
    :type temporary: bool
    :returns: A buffered writable file descriptor
    :rtype: BufferedFileStorage

renders correctly if params.rst contains additional parameter definitions in the same style. When trying to do the same with Google docstrings + napoleon, i.e.

   Args:
        .. include:: params.rst
        file_id (:obj:`str`): Identifier for this file, which can be used to download
            or reuse the file.

this will render to a parameter include: (.) – params.rst:

Similarly, adding something like .. include:: note.rst in the docstring but outside of the Args: will render the note contained in note.rst correctly only if it's given as .. note:: … instead of Note: …

Describe the solution you'd like It would be nice if directives would be resolved before napoleon parses the docstrings so that these kinds of things would work (also with other & custom directives)

Describe alternatives you've considered I tried inserting the text via a SphinxRole subclass, but realized that for that I would have to parse the docstring manually, so I gave up on that

Additional context I haven't really understood the order in which sphinx reads the files, reads docstrings via autodoc, resolves directives, and applies napoleon or how that order is determined. If you can point me in the right direction, I'd be happy to try & PR for this.

Bibo-Joshi commented 2 years ago

Did some more digging and I guess I understand now why this doesn't work. IISC the order of things is as roughly follows:

  1. sphinx reads a docstrings
  2. the docstring is passed to napoleon as is, i.e. as list of the lines of the docstring as strings
  3. napoleon edits that list in place effectively inserting native docutils directives
  4. docutils parses the edited list splitting everything up into nodes, resolving directives (including .. include:: etc)
  5. sphinx renders everything

The important part is that napoleon works on the docstrings before it passes through docutils. From my understanding that means that there is no easy solution without completely reworking napoleon to work on the output of docutils instead.

E.g. I first thought that overridig sphinx.directives.other.Include.run to manually napoleon-parse the included lines could work. But that will probably quickly reach its limits in my above parameter example because napoleon doesn't find Args: and hence probably can't guess if the included line should be converted.

Now ofc this is a somewhat special usecase, so I'd be willing to type the docutils parameter definitions instead - but unfortunately that doesn't work either. The issue is that napoleon thinks that .. include:: params.rst is a parameter and converts it into :param .. include:: params.rst:. This however should be relatively easy to avoid - as proof of concept a simple regex-replace at the end of sphinx.ext.napoleon._process_docstring did the trick just fine.

If my original request is too complicated & special case to consider, I'd like to ask to at least make napoleon properly handle .. include:: (and probably also other directives) in parameter & attribute lists :)

tk0miya commented 2 years ago

Yes, your understanding is correct. As you said, it's difficult to support your request.

If my original request is too complicated & special case to consider, I'd like to ask to at least make napoleon properly handle .. include:: (and probably also other directives) in parameter & attribute lists :)

It means we have to reinvent the docutils parsers and directives only for napoleon because we can't use existing docutils parsers and directives (they return doctree instead of text!). So we need to create an "include" directive that returns text block instead.

Additionally, we'll receive requests for supporting other directives if we'll support the "include" directive once. It must be a tough topic for us.

Bibo-Joshi commented 2 years ago

Yes, your understanding is correct. As you said, it's difficult to support your request.

All right, thanks for the confirmation!

If my original request is too complicated & special case to consider, I'd like to ask to at least make napoleon properly handle .. include:: (and probably also other directives) in parameter & attribute lists :)

It means we have to reinvent the docutils parsers and directives only for napoleon because we can't use existing docutils parsers and directives (they return doctree instead of text!). So we need to create an "include" directive that returns text block instead.

Additionally, we'll receive requests for supporting other directives if we'll support the "include" directive once. It must be a tough topic for us.

Sorry, I should have made myself more clear. What I meant is that if resolving directives before running napoleon is too complicated, then it would be nice if napoleon didn't try to convert directives into docutils synatx. I.e. when running napeolen, it would convert

Args:
    .. directive:: (input)
      :option:
    arg (type): description

into

.. directive:: (input)
   :option:
:param arg: description
:type arg: type

instead of the current output

:param .. directive:: (input): :option:
:param arg: description
:type arg: type

I didn't go through the internals of Goole/NumpyDocString yet, but I guess that should be possible with some regex (napeoleon apparently uses regex a lot anyway :D). then after napeolon is done, docutils would still be able to correctly pick up the directives. While trying to figure out the mechanics, achieved this by editing sphinx.ext.napoleon._process_docstring to do

lines[:] = (line.replace(':param .. include:: params.rst:', '.. include:: params.rst') for line in result_lines)