Qiskit / documentation

The documentation content home for https://docs.quantum.ibm.com.
https://docs.quantum.ibm.com
Apache License 2.0
38 stars 81 forks source link

Dedicated class pages are missing type hints for properties because of autosummary `.. autoproperty::` quirk #2346

Open Eric-Arellano opened 2 days ago

Eric-Arellano commented 2 days ago

Problem: missing type hints

With Sphinx's autosummary extension, we're using .. autoattribute:: for functions marked with @property, rather than .. autoproperty:::

https://github.com/Qiskit/qiskit-addon-obp/blob/2412cb29c9d314a5ad568acdd5be558f7053c0f2/docs/_templates/autosummary/class.rst?plain=1#L20

This messes up the return type for properties. For example, the return type is defined on this @property:

https://github.com/Qiskit/qiskit-addon-mpf/blob/191dc8162510719ca3b50e5a25e9a1453419629a/qiskit_addon_mpf/backends/tenpy_tebd/evolver.py#L59-L62

However, the Sphinx output—and thus our docs—are missing that return type:

https://github.com/Qiskit/documentation/blob/298d8a3fcf84c741cd04ab372db4f4ee9dd2bfc7/docs/api/qiskit-addon-mpf/backends-tenpy-tebd-tebd-evolver.mdx?plain=1#L40-L44

This is fixed if we use .. autoproperty::; Sphinx will correctly set the return type.

So, why not use .. autoproperty::?

This is specifically an issue when we use .. autosummary:: to document a class. Properties work correctly when you use .. autoclass::, such as for inline class documentation in a module page.

The issue is that our template tells .. autoclass:: to not document any members so that we can instead manually document them by calling .. autoattribute and .. automethod:

.. currentmodule:: {{ module }}

.. autoclass:: {{ objname }}
   :no-members:
   :no-inherited-members:
   :no-special-members:
   :show-inheritance:

{% block attributes_summary %}
  {% if attributes %}
   .. rubric:: Attributes
    {% for item in attributes %}
   .. autoattribute:: {{ item }}
    {%- endfor %}
  {% endif %}
{% endblock -%}

{% block methods_summary %}
  {% set wanted_methods = (methods | reject('==', '__init__') | list) %}
  {% if wanted_methods %}
   .. rubric:: Methods
    {% for item in wanted_methods %}
   .. automethod:: {{ item }}
    {%- endfor %}
  {% endif %}
{% endblock %}

In our Autosummary template, we manually list out every member so that we can add the lines .. rubric:: Attributes and .. rubric:: Methods, which get converted into h2 headings. These headings give important hierarchy to class pages that we do not want to lose:

Image

So, we cannot simply use .. autoclass:: to list all the members for us because we would lose the Attributes and Methods headings.

Ideally, we could do something like this:

{% block attributes_summary %}
  {% if attributes %}
   .. rubric:: Attributes
    {% for item in attributes %}
       {% if item is property %}
   .. autoproperty:: {{ item}}
       {% else %}
   .. autoattribute:: {{ item }}
       {% endif %}
    {%- endfor %}
  {% endif %}
{% endblock -%}

However, that type of if statement will not work because attributes is simply a list of strings, with both normal attributes and properties, such as ['foo', 'bar', 'my_prop']. There is no way in the Jinja template to determine whether the string corresponds to an @property, and Sphinx fails if you try using any Python builtins like dir() or hasattr(). What we really would need is for Autosummary to inject a properties variable, but it's not available: https://www.sphinx-doc.org/en/master/usage/extensions/autosummary.html#customizing-templates.

Rejected workaround: manually list every class

This wouldn't be a problem if we had docs authors give up on autosummary and manually call .. autoclass::, .. autoproperty::, .. autoattribute::, and .. automethod::, along with adding .. rubric:: Attributes and .. rubric:: Methods where appropriate. However, that is not realistic to expect.

Proposed solution: our script

We will switch the APIs to use .. autoclass:: in their Autosummary template to document their members, rather than manually iterating through them. Their template will look like this:

.. currentmodule:: {{ module }}

.. autoclass:: {{ objname }}
   :show-inheritance:

That will fix the property issue, but now we'll be missing the Attributes and Methods h2 headings! So, we will have our qiskit/documentation script add back those headings to our MDX files in the appropriate location.

TBD: