Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Resolve directives before running napoleon #9939

Open
Bibo-Joshi opened this issue Dec 5, 2021 · 3 comments
Open

Resolve directives before running napoleon #9939

Bibo-Joshi opened this issue Dec 5, 2021 · 3 comments
Labels
extensions:napoleon type:enhancement enhance or introduce a new feature

Comments

@Bibo-Joshi
Copy link
Contributor

Bibo-Joshi commented Dec 5, 2021

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
Copy link
Contributor Author

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
Copy link
Member

tk0miya commented Jan 17, 2022

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
Copy link
Contributor Author

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)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
extensions:napoleon type:enhancement enhance or introduce a new feature
Projects
None yet
Development

No branches or pull requests

3 participants