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.

Sign up for GitHub

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

Jump to bottom

Always show type hints in docs #31

Merged
merged 1 commit into from
Nov 7, 2024
Merged

Always show type hints in docs #31

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions docs/_templates/autosummary/class.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
.. autoclass:: {{ objname }}
:no-members:
:no-inherited-members:
:no-special-members:
:show-inheritance:

{% block attributes_summary %}
Expand Down
5 changes: 0 additions & 5 deletions docs/_templates/autosummary/function.rst
View file
Open in desktop
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure why this was added. None of the main Qiskit repos have it. It didn't change how qiskit/documentation works

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was added to ensure reasonable titles for docs pages which only contain a function.
Below is a before and after of the docs page for qiskit_addon_obp.backpropagate:

With function.rst present:
screenshot_1730796916

Without it:
screenshot_1730796909

Notice the change in both the title and the navigation column on the left. Especially the latter I find very cumbersome as the fully resolved module names can become very long.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. For what it's worth, it makes zero impact on how docs.quantum.ibm.com is rendered. I tested this out by first un-inlining a function & generating the docs w/o the template. Then I added back this custom template & regenerated - no change.

We show the full API name in the h1 for SEO purposes and so people know the import path. The left TOC looks like this:

Screenshot 2024-11-06 at 4 42 10 PM

So, I don't really have a preference because it doesn't impact docs.quantum.ibm.com. What would you prefer me to do?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay I see. If it has no effect I suppose it does not matter and you can leave it like this. I assume we won't be publishing the docs in two places.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume we won't be publishing the docs in two places.

Exactly. There will be a transition period for ~1-2 months as we migrate your guides to docs.quantum.ibm.com. Once that's done, we'll redirect GitHub Pages to the IBM site.

This file was deleted.

6 changes: 3 additions & 3 deletions docs/conf.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,16 +74,16 @@
.. |version| replace:: {release}
"""

# Options for autodoc. These reflect the values from Terra.
# Options for autodoc. These reflect the values from Qiskit SDK and Runtime.
autosummary_generate = True
autosummary_generate_overwrite = False
autoclass_content = "both"
autodoc_typehints = "description"
autodoc_typehints_description_target = "documented_params"
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the main change

autodoc_member_order = "bysource"
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I got rid of this because I think the default of alphabetical is more sensical, and it's what all the other non-addon projects use. You can read more at https://www.sphinx-doc.org/en/master/usage/extensions/autodoc.html#confval-autodoc_member_order. Feel free to disagree!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't remember for which specific case we added this but I am fine with going back to alphabetical

autodoc_default_options = {
"inherited-members": None,
}
napoleon_google_docstring = True
napoleon_numpy_docstring = False
Comment on lines +85 to +86
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small performance optimization & reduces risk of unintended behavior



# This adds numbers to the captions for figures, tables,
Expand Down