-
Notifications
You must be signed in to change notification settings - Fork 81
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
Dedicated class pages are missing type hints for properties because of autosummary .. autoproperty::
quirk
#2346
Comments
Eric-Arellano
changed the title
Autosummary template needs to use
Dedicated class pages are missing type hints for properties because of autosummary Nov 21, 2024
.. autoproperty::
for properties.. autoproperty::
quirk
Eric-Arellano
added a commit
to Qiskit/qiskit-ibm-runtime
that referenced
this issue
Nov 25, 2024
Closes #1877. We now use `EstimatorPubLike` and `SamplerPubLike` for the type hints, rather than Sphinx fully expanding the types into extremely long values shown in #1877. To fix the type hints, we have to stop using `sphinx-autodoc-typehints` and instead use autodoc's builtin support for type hints. I didn't realize we were still using `sphinx-autodoc-typehints` - the other Qiskit projects migrated off a while ago. This PR also makes some other improvements to align with changes we made in Qiskit Addons & qiskit-ibm-transpiler: * always show inheritance in class pages for the base class * index page should show the PyPI package name in code syntax * don't show signatures in `autosummary` tables, since they're noisy * show type hints for parameters that don't have docstring ## Introduces a regression: type hints for properties Unfortunately, switching off of `sphinx-autodoc-typehints` results in Qiskit/documentation#2346. This is a quirk of autosummary that we cannot easily fix. We have a plan to fix it, although realistically won't get to it until Q1 and possibly Q2. In the meantime, consider repeating some of the type information in the docstring. For example, in ``` <Attribute id="qiskit_ibm_runtime.EstimatorV2.mode"> Return the execution mode used by this primitive. ``` You could rewrite to say ``` <Attribute id="qiskit_ibm_runtime.EstimatorV2.mode"> Return the execution mode used by this primitive, either :class:~`.Session` or ~:class:~`.Batch`. ```
Eric-Arellano
added a commit
to Eric-Arellano/qiskit-ibm-runtime
that referenced
this issue
Nov 25, 2024
Closes Qiskit#1877. We now use `EstimatorPubLike` and `SamplerPubLike` for the type hints, rather than Sphinx fully expanding the types into extremely long values shown in Qiskit#1877. To fix the type hints, we have to stop using `sphinx-autodoc-typehints` and instead use autodoc's builtin support for type hints. I didn't realize we were still using `sphinx-autodoc-typehints` - the other Qiskit projects migrated off a while ago. This PR also makes some other improvements to align with changes we made in Qiskit Addons & qiskit-ibm-transpiler: * always show inheritance in class pages for the base class * index page should show the PyPI package name in code syntax * don't show signatures in `autosummary` tables, since they're noisy * show type hints for parameters that don't have docstring ## Introduces a regression: type hints for properties Unfortunately, switching off of `sphinx-autodoc-typehints` results in Qiskit/documentation#2346. This is a quirk of autosummary that we cannot easily fix. We have a plan to fix it, although realistically won't get to it until Q1 and possibly Q2. In the meantime, consider repeating some of the type information in the docstring. For example, in ``` <Attribute id="qiskit_ibm_runtime.EstimatorV2.mode"> Return the execution mode used by this primitive. ``` You could rewrite to say ``` <Attribute id="qiskit_ibm_runtime.EstimatorV2.mode"> Return the execution mode used by this primitive, either :class:~`.Session` or ~:class:~`.Batch`. ```
kt474
pushed a commit
to Qiskit/qiskit-ibm-runtime
that referenced
this issue
Nov 25, 2024
) Closes #1877. We now use `EstimatorPubLike` and `SamplerPubLike` for the type hints, rather than Sphinx fully expanding the types into extremely long values shown in #1877. To fix the type hints, we have to stop using `sphinx-autodoc-typehints` and instead use autodoc's builtin support for type hints. I didn't realize we were still using `sphinx-autodoc-typehints` - the other Qiskit projects migrated off a while ago. This PR also makes some other improvements to align with changes we made in Qiskit Addons & qiskit-ibm-transpiler: * always show inheritance in class pages for the base class * index page should show the PyPI package name in code syntax * don't show signatures in `autosummary` tables, since they're noisy * show type hints for parameters that don't have docstring ## Introduces a regression: type hints for properties Unfortunately, switching off of `sphinx-autodoc-typehints` results in Qiskit/documentation#2346. This is a quirk of autosummary that we cannot easily fix. We have a plan to fix it, although realistically won't get to it until Q1 and possibly Q2. In the meantime, consider repeating some of the type information in the docstring. For example, in ``` <Attribute id="qiskit_ibm_runtime.EstimatorV2.mode"> Return the execution mode used by this primitive. ``` You could rewrite to say ``` <Attribute id="qiskit_ibm_runtime.EstimatorV2.mode"> Return the execution mode used by this primitive, either :class:~`.Session` or ~:class:~`.Batch`. ```
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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:
documentation/docs/api/qiskit-addon-mpf/backends-tenpy-tebd-tebd-evolver.mdx
Lines 40 to 44 in 298d8a3
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
: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:So, we cannot simply use
.. autoclass::
to list all the members for us because we would lose theAttributes
andMethods
headings.Ideally, we could do something like this:
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 likedir()
orhasattr()
. What we really would need is for Autosummary to inject aproperties
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:That will fix the property issue, but now we'll be missing the
Attributes
andMethods
h2 headings! So, we will have our qiskit/documentation script add back those headings to our MDX files in the appropriate location.TBD:
Attributes
andMethods
headings for inline classes on module pages.The text was updated successfully, but these errors were encountered: