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

Parameter Names and Types are Displayed Wrong in Docs #468

Closed
Mr0grog opened this issue Aug 29, 2019 · 3 comments · Fixed by #489
Closed

Parameter Names and Types are Displayed Wrong in Docs #468

Mr0grog opened this issue Aug 29, 2019 · 3 comments · Fixed by #489

Comments

@Mr0grog
Copy link
Member

Mr0grog commented Aug 29, 2019

In a recent Sphinx or Numpydoc update, our docs stopped differentiating between a method’s parameters and their types, so they get joined up with no spaces in the display:

Screen Shot 2019-08-29 at 8 47 28 AM

They used to display like this:

Screen Shot 2019-10-04 at 12 35 42 PM

(Updated 2019-10-04 to cross out the incorrect analysis below. See the first comment (#468 (comment)) for correct info.)


Old, incorrect analysis:

We think there was an update that switched the parameter documentation style from:

Parameters
----------
page_id : string
    Page to which the Version is associated

To:

Parameters
----------
page_id: string
    Page to which the Version is associated

Note the missing space between the parameter name and the colon. So we probably just need to update all the docstrings throughout the codebase to remove those spaces.

@Mr0grog
Copy link
Member Author

Mr0grog commented Oct 4, 2019

Update: the solution described above is completely wrong. 🙁

What’s actually happening here appears to be an incompatibility between Sphinx v2 and the sphinx_rtd_theme plugin. Numpydoc is still doing the right thing, and there are no changes or issues with how our actual docstrings are getting parsed.

You can skip to the conclusion at the bottom, but if you want a full overview of what’s broken, read along with my journey into our docs tooling…

Some Background on Our Doc Build Tools

This was a journey — I’ve never dug into Sphinx before, and there are a bunch of pieces to making this work.

  • Sphinx is the overarching system that builds our docs. It does some pieces itself, but also supports a plugin and theming system that takes care of a lot of the work. Sphinx mostly just knows how to interpret reStructuredText (.rst files).

  • Autodoc is a Sphinx plugin that generates Sphinx documentation for Python objects from their docstrings in Python code. It mainly just pulls the docstrings into Sphinx pages, so your docstrings have to be in reStructuredText format.

  • Numpy is a Sphinx plugin that messes with Autodoc. When Autodoc signals that it’s loading the docstring for an object, Numpy jumps in and parses that docstring if it’s written in the Numpy style (which comes from the Numpy Python project), and returns reStructedText for Autodoc to use.

  • sphinx_rtd_theme is a Sphinx theme that makes HTML output looks like the standard style on https://readthedocs.io.

So the docs pipeline when you run make html looks something like:

 ┌────────────────────────────────────────────────────────────────────────────┐
 │                                Sphinx                                      │
 └────────────────────────────────────────────────────────────────────────────┘
        │                      ↑               |           ↑             ↓
   Encounters an            Returns           HTML        HTML        Write to
 autodoc directive         .rst text        fragments     pages        Disk
        ↓                      │               ↓           |             ↓
    ┌──────────────────────────────┐       ┌──────────────────┐        Done!
    │           Autodoc            │       │ sphinx_rtd_theme │
    └──────────────────────────────┘       └──────────────────┘
           │                ↑
    Signals parsing      Returns
      a docstring       .rst text
           ↓                │
         ┌────────────────────┐
         │       Numpy        │
         └────────────────────┘

Why the Proposed Fix Above is Wrong

Based on the above, there are a few points where things could be going wrong.

  1. Numpy parsing broke/changed. (Nope, it produces the same results as before.
  2. Autodoc now expects something different. (Nope, looking into autodoc and numpy, it’s clear Numpy was never producing anything autodoc does or did have to interpret specially. Nothing has changed here.)
  3. sphinx_rtd_theme is formatting the data from Sphinx wrong. (Yes and No. It hasn’t changed and the same version of sphinx_rtd does the right thing with Sphinx v1 and the wrong thing with Sphinx v2. It doesn’t format the pieces of a function definition; it just gets them as HTML fragments from Sphinx.)
  4. Sphinx is converting reStructuredText to HTML differently. (Yes!)

So the actual problem here is a mix of 4 (how Sphinx renders HTML fragments from .rst) and 3 (how sphinx_rtd_theme formats and styles those fragments). More importantly, the issue is clearly not 1 (Numpy), so changing our docstrings isn’t the right solution.

In fact, changing the docstrings to remove the space between the parameter name and colon as described in the original post will make the colon show up in the docs, but makes the underlying problem worse.

Take the docstring for db.Client.list_changes:

def list_changes(self, page_id, include_total=False):
"""
List Changes between two Versions on a Page.
Parameters
----------
page_id : string
include_total : boolean, optional
Whether to include a `meta.total_results` field in the response.
If not set, `links.last` will usually be empty unless you are on
the last chunk. Setting this option runs a pretty expensive query,
so use it sparingly. (Default: False)
Returns
-------
response : dict
"""

Numpy parses it to .rst like so:

List Changes between two Versions on a Page.

:Parameters:

    **page_id** : string
        ..

    **include_total** : boolean, optional
        Whether to include a `meta.total_results` field in the response.
        If not set, `links.last` will usually be empty unless you are on
        the last chunk. Setting this option runs a pretty expensive query,
        so use it sparingly. (Default: False)

:Returns:

    **response** : dict
        ..

And if we remove the space between parameter name and the colon, it parses like this:

List Changes between two Versions on a Page.

:Parameters:

    **page_id: string**
        ..

    **include_total: boolean, optional**
        Whether to include a `meta.total_results` field in the response.
        If not set, `links.last` will usually be empty unless you are on
        the last chunk. Setting this option runs a pretty expensive query,
        so use it sparingly. (Default: False)

:Returns:

    **response: dict**
        ..

The parameters in both cases are definition lists. Definition lists in .rst get converted to <dl> elements in HTML, but they have a special extra feature HTML definition lists do not: each term can be annotated with a classifier (definition list docs). When we remove the space before the colon, we changed the type information from a classifier to just being part of the parameter name (the “term” in the definition list). We get the colon shown in the result, but can no longer differentiate formatting between the parameter name and its type, optionality, etc. That’s bad.

So what’s the real problem and what should we actually be doing?

Conclusion (TL;DR)

The real issue is that Sphinx v1 and v2 render reStructedText field lists and definition lists in very different ways, and sphinx_rtd_theme is designed to work with the way v1 did it:

  • Field lists: In v1, field lists were tables, with field names (e.g. “parameters” or “returns”) in the first column and their contents (e.g. the list of parameters, return values, etc.) in the second:

    <table class="docutils field-list">
        <colgroup><col class="field-name"><col class="field-body"></colgroup>
        <tbody valign="top">
            <tr class="field-odd field">
                <th class="field-name">Parameters:</th>
                <td class="field-body">
                    <!-- Contents of parameters section -->
                </td>
            </tr>
        </tbody>
    </table>

    In v2, they are definition lists (<dl>), with the field name as the term (<dt>) and their contents as the description (<dd>):

    <dl class="field-list simple">
        <dt class="field-odd">Parameters</dt>
        <dd class="field-odd">
            <!-- Contents of parameters section -->
        </dd>
    </dl>

    This is why the field names (e.g. the “parameters” heading) are no longer to the left of their contents and why they are formatted like individual parameter names were — their markup is totally different and collides with how definition lists for the individual parameters handled in v1 (see below). On the upside, this markup is much lighter and more flexible to style. (Although it makes it hard to group the field name and its content.)

  • Definition lists: In v1, a definition list term and its classifier were separated by a <span> with a colon in it:

    <dl> class="docutils">
        <dt>
            term <span class="classifier-delimiter">:</span> <span class="classifier">classifier text</span>
        </dt>
        <dd>
            <!-- Description -->
        </dd>
    </dl>

    In v2, a definition list term and its classifier are not separated at all, and a theme is expected to use CSS generated content or other styling to separate the term and its classifier:

    <dl class="simple">
        <dt>
            term<span class="classifier">classifier text</span>
        </dt>
        <dd>
            <!-- Description -->
        </dd>
    </dl>
    <!-- Sphinx doesn't output this, but expects themes to do something like: -->
    <style>
        .classifier::before {
            content: ": ";
        }
    </style>

    This is why we’re missing the colon and space between the parameter names and their types. This setup is more flexible for styling, though.

So! The basic solution here is to add some extra CSS to our output (or just to downgrade to Sphinx v1). Read The Docs has some info on this: https://docs.readthedocs.io/en/latest/guides/adding-custom-css.html

We need to add the .classifier::before as described for definition lists above, and will need to come up with some nicer styling for the the field lists, which will be a bit nuanced since it’s also marked up as a <dl>. You can find the styling for sphinx_rtd_theme that you’ll be adding onto here: https://github.com/readthedocs/sphinx_rtd_theme/blob/master/src/sass/_theme_rst.sass


Relevant issues at other projects:

mrotondo added a commit to mrotondo/web-monitoring-processing that referenced this issue Oct 31, 2019
Mr0grog pushed a commit that referenced this issue Nov 5, 2019
The Read-the-Docs theme we use for our docs is not yet totally compatible with Sphinx v2, and causes a function’s parameter name and types to get smushed together and look confusing. This adds some custom CSS to fix that up.

Fixes #468.
@ZhengRui
Copy link

ZhengRui commented Aug 8, 2020

Update: the solution described above is completely wrong. 🙁

What’s actually happening here appears to be an incompatibility between Sphinx v2 and the sphinx_rtd_theme plugin. Numpydoc is still doing the right thing, and there are no changes or issues with how our actual docstrings are getting parsed.

You can skip to the conclusion at the bottom, but if you want a full overview of what’s broken, read along with my journey into our docs tooling…

Some Background on Our Doc Build Tools

This was a journey — I’ve never dug into Sphinx before, and there are a bunch of pieces to making this work.

  • Sphinx is the overarching system that builds our docs. It does some pieces itself, but also supports a plugin and theming system that takes care of a lot of the work. Sphinx mostly just knows how to interpret reStructuredText (.rst files).
  • Autodoc is a Sphinx plugin that generates Sphinx documentation for Python objects from their docstrings in Python code. It mainly just pulls the docstrings into Sphinx pages, so your docstrings have to be in reStructuredText format.
  • Numpy is a Sphinx plugin that messes with Autodoc. When Autodoc signals that it’s loading the docstring for an object, Numpy jumps in and parses that docstring if it’s written in the Numpy style (which comes from the Numpy Python project), and returns reStructedText for Autodoc to use.
  • sphinx_rtd_theme is a Sphinx theme that makes HTML output looks like the standard style on https://readthedocs.io.

So the docs pipeline when you run make html looks something like:

 ┌────────────────────────────────────────────────────────────────────────────┐
 │                                Sphinx                                      │
 └────────────────────────────────────────────────────────────────────────────┘
        │                      ↑               |           ↑             ↓
   Encounters an            Returns           HTML        HTML        Write to
 autodoc directive         .rst text        fragments     pages        Disk
        ↓                      │               ↓           |             ↓
    ┌──────────────────────────────┐       ┌──────────────────┐        Done!
    │           Autodoc            │       │ sphinx_rtd_theme │
    └──────────────────────────────┘       └──────────────────┘
           │                ↑
    Signals parsing      Returns
      a docstring       .rst text
           ↓                │
         ┌────────────────────┐
         │       Numpy        │
         └────────────────────┘

Why the Proposed Fix Above is Wrong

Based on the above, there are a few points where things could be going wrong.

  1. Numpy parsing broke/changed. (Nope, it produces the same results as before.
  2. Autodoc now expects something different. (Nope, looking into autodoc and numpy, it’s clear Numpy was never producing anything autodoc does or did have to interpret specially. Nothing has changed here.)
  3. sphinx_rtd_theme is formatting the data from Sphinx wrong. (Yes and No. It hasn’t changed and the same version of sphinx_rtd does the right thing with Sphinx v1 and the wrong thing with Sphinx v2. It doesn’t format the pieces of a function definition; it just gets them as HTML fragments from Sphinx.)
  4. Sphinx is converting reStructuredText to HTML differently. (Yes!)

So the actual problem here is a mix of 4 (how Sphinx renders HTML fragments from .rst) and 3 (how sphinx_rtd_theme formats and styles those fragments). More importantly, the issue is clearly not 1 (Numpy), so changing our docstrings isn’t the right solution.

In fact, changing the docstrings to remove the space between the parameter name and colon as described in the original post will make the colon show up in the docs, but makes the underlying problem worse.

Take the docstring for db.Client.list_changes:

def list_changes(self, page_id, include_total=False):
"""
List Changes between two Versions on a Page.
Parameters
----------
page_id : string
include_total : boolean, optional
Whether to include a `meta.total_results` field in the response.
If not set, `links.last` will usually be empty unless you are on
the last chunk. Setting this option runs a pretty expensive query,
so use it sparingly. (Default: False)
Returns
-------
response : dict
"""

Numpy parses it to .rst like so:

List Changes between two Versions on a Page.

:Parameters:

    **page_id** : string
        ..

    **include_total** : boolean, optional
        Whether to include a `meta.total_results` field in the response.
        If not set, `links.last` will usually be empty unless you are on
        the last chunk. Setting this option runs a pretty expensive query,
        so use it sparingly. (Default: False)

:Returns:

    **response** : dict
        ..

And if we remove the space between parameter name and the colon, it parses like this:

List Changes between two Versions on a Page.

:Parameters:

    **page_id: string**
        ..

    **include_total: boolean, optional**
        Whether to include a `meta.total_results` field in the response.
        If not set, `links.last` will usually be empty unless you are on
        the last chunk. Setting this option runs a pretty expensive query,
        so use it sparingly. (Default: False)

:Returns:

    **response: dict**
        ..

The parameters in both cases are definition lists. Definition lists in .rst get converted to <dl> elements in HTML, but they have a special extra feature HTML definition lists do not: each term can be annotated with a classifier (definition list docs). When we remove the space before the colon, we changed the type information from a classifier to just being part of the parameter name (the “term” in the definition list). We get the colon shown in the result, but can no longer differentiate formatting between the parameter name and its type, optionality, etc. That’s bad.

So what’s the real problem and what should we actually be doing?

Conclusion (TL;DR)

The real issue is that Sphinx v1 and v2 render reStructedText field lists and definition lists in very different ways, and sphinx_rtd_theme is designed to work with the way v1 did it:

  • Field lists: In v1, field lists were tables, with field names (e.g. “parameters” or “returns”) in the first column and their contents (e.g. the list of parameters, return values, etc.) in the second:

    <table class="docutils field-list">
        <colgroup><col class="field-name"><col class="field-body"></colgroup>
        <tbody valign="top">
            <tr class="field-odd field">
                <th class="field-name">Parameters:</th>
                <td class="field-body">
                    <!-- Contents of parameters section -->
                </td>
            </tr>
        </tbody>
    </table>
    

    In v2, they are definition lists (<dl>), with the field name as the term (<dt>) and their contents as the description (<dd>):

    <dl class="field-list simple">
        <dt class="field-odd">Parameters</dt>
        <dd class="field-odd">
            <!-- Contents of parameters section -->
        </dd>
    </dl>
    

    This is why the field names (e.g. the “parameters” heading) are no longer to the left of their contents and why they are formatted like individual parameter names were — their markup is totally different and collides with how definition lists for the individual parameters handled in v1 (see below). On the upside, this markup is much lighter and more flexible to style. (Although it makes it hard to group the field name and its content.)

  • Definition lists: In v1, a definition list term and its classifier were separated by a <span> with a colon in it:

    <dl> class="docutils">
        <dt>
            term <span class="classifier-delimiter">:</span> <span class="classifier">classifier text</span>
        </dt>
        <dd>
            <!-- Description -->
        </dd>
    </dl>
    

    In v2, a definition list term and its classifier are not separated at all, and a theme is expected to use CSS generated content or other styling to separate the term and its classifier:

    <dl class="simple">
        <dt>
            term<span class="classifier">classifier text</span>
        </dt>
        <dd>
            <!-- Description -->
        </dd>
    </dl>
    <!-- Sphinx doesn't output this, but expects themes to do something like: -->
    <style>
        .classifier::before {
            content: ": ";
        }
    </style>
    

    This is why we’re missing the colon and space between the parameter names and their types. This setup is more flexible for styling, though.

So! The basic solution here is to add some extra CSS to our output (or just to downgrade to Sphinx v1). Read The Docs has some info on this: https://docs.readthedocs.io/en/latest/guides/adding-custom-css.html

We need to add the .classifier::before as described for definition lists above, and will need to come up with some nicer styling for the the field lists, which will be a bit nuanced since it’s also marked up as a <dl>. You can find the styling for sphinx_rtd_theme that you’ll be adding onto here: https://github.com/readthedocs/sphinx_rtd_theme/blob/master/src/sass/_theme_rst.sass

Relevant issues at other projects:

@Mr0grog Thank you for detailed explanation. I have sphinx 3.2.0, sphinx-rtd-theme 0.5.0 installed, still for field list like Parameters, Returns it is rendered with in <table class="docutils field-list"></table>, instead of <dl></dl>. I am very confused if this is an issue of sphinx or the issue of sphinx-rtd-theme, readthedocs/sphinx_rtd_theme#766 (comment) has the same question as mine.

I want to achieve this style as show in sphinx-rtd-theme documentation page:
image

But what I have is like this:
image

Can you provide any insights on what could be wrong on my settings? Thanks 🙏

@ZhengRui
Copy link

ZhengRui commented Aug 8, 2020

I was able to solve it using some hack of css

readthedocs/sphinx_rtd_theme#766 (comment)

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

Successfully merging a pull request may close this issue.

2 participants