-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Use sphinx.ext.linkcode
for more precise source code links (backport #11851)
#12146
Conversation
* switched from sphinx.ext.viewcode to sphinx.ext.linkcode * removed extra line * Add section header for source code links Co-authored-by: Eric Arellano <[email protected]> * removed docstring Co-authored-by: Eric Arellano <[email protected]> * update return string Co-authored-by: Eric Arellano <[email protected]> * added back blank line * Added a method to determine the GitHub branch Co-authored-by: Eric Arellano <[email protected]> * add blank line Co-authored-by: Eric Arellano <[email protected]> * remove print statement Co-authored-by: Eric Arellano <[email protected]> * Try to fix error for contextlib file We got this stacktrace: Traceback (most recent call last): File "/home/vsts/work/1/s/.tox/docs/lib/python3.8/site-packages/sphinx/events.py", line 96, in emit results.append(listener.handler(self.app, *args)) File "/home/vsts/work/1/s/.tox/docs/lib/python3.8/site-packages/sphinx/ext/linkcode.py", line 55, in doctree_read uri = resolve_target(domain, info) File "/home/vsts/work/1/s/docs/conf.py", line 216, in linkcode_resolve file_name = PurePath(full_file_name).relative_to(repo_root) File "/opt/hostedtoolcache/Python/3.8.18/x64/lib/python3.8/pathlib.py", line 908, in relative_to raise ValueError("{!r} does not start with {!r}" ValueError: '/opt/hostedtoolcache/Python/3.8.18/x64/lib/python3.8/contextlib.py' does not start with '/home/vsts/work/1/s' We shouldn't even attempt to generate a link for the file contextlib.py * Try to fix error for Jenkins run #20240221.52 New build failed with this stacktrace: Traceback (most recent call last): File "/home/vsts/work/1/s/.tox/docs/lib/python3.8/site-packages/sphinx/events.py", line 96, in emit results.append(listener.handler(self.app, *args)) File "/home/vsts/work/1/s/.tox/docs/lib/python3.8/site-packages/sphinx/ext/linkcode.py", line 55, in doctree_read uri = resolve_target(domain, info) File "/home/vsts/work/1/s/docs/conf.py", line 215, in linkcode_resolve full_file_name = inspect.getsourcefile(obj) File "/opt/hostedtoolcache/Python/3.8.18/x64/lib/python3.8/inspect.py", line 696, in getsourcefile filename = getfile(object) File "/opt/hostedtoolcache/Python/3.8.18/x64/lib/python3.8/inspect.py", line 665, in getfile raise TypeError('{!r} is a built-in class'.format(object)) TypeError: <class 'builtins.CustomClassical'> is a built-in class So I added a condition that the obj should not be a built-in * Check that qiskit in module name sooner * moved valid code object verification earlier * added try except statement to getattr call * added extra try/except block * Also support Azure Pipelines * removed unused import * Revert Azure support to keep things simple * added extra "/" to final URL * Move GitHub branch logic to GitHub Action * switched to importlib and removed redundant block of code * Apply suggestions from code review Co-authored-by: Eric Arellano <[email protected]> * added back spaces * Clarify docs_deploy GitHub logic 1. Remove misleading PR conditional. This worfklow doesn't even run in PRs. It was bad copy-pasta from the runtime repo 2. Clarify why we point tag builds to their stable branch name * Use pathlib for relativizing file name * Fix relative_to() path * Remove tox prefix --------- Co-authored-by: Eric Arellano <[email protected]> (cherry picked from commit 97788f0)
Thank you for opening a new pull request. Before your PR can be merged it will first need to pass continuous integration tests and be reviewed. Sometimes the review process can be slow, so please be patient. While you're waiting, please feel free to review other open PRs. While only a subset of people are authorized to approve pull requests for merging, everyone is encouraged to review open pull requests. Doing reviews helps reduce the burden on the core team and helps make the project's code better for everyone. One or more of the the following people are requested to review this:
|
Pull Request Test Coverage Report for Build 8573060548Details
💛 - Coveralls |
Summary
This addresses Qiskit/documentation#517 by switching out the sphinx.ext.viewcode for the sphinx.ext.linkcode extension which allows our GitHub links in the documentation to be linked to the specific lines of code, not just the file. This was tested successfully in Qiskit/qiskit_sphinx_theme#589 so now we want to implement it in the qiskit, runtime, and provider repos.
Example links generated in this PR build:
The API generation script at qiskit/documentation already knows how to handle
sphinx.ext.linkcode
.Determining the GitHub branch
We need to detect when to use
main
vs stable branches likestable/1.0
, particularly for stable docs builds with GitHub Actions that get used by qiskit/documentation to deploy the docs. PR and local builds are less critical because the docs don't get published and are only for previews by code authors.We do this with GitHub Actions environment variables. The docs workflow sets the env variable
QISKIT_DOCS_GITHUB_BRANCH_NAME
read by Sphinx inconf.py
. You can see the workflow correctly passing the value to Sphinx in this example run.Examples of the workflow logic:
PR builds use
main
PR builds and the merge queue use Azure Pipelines. To keep this infrastructure simpler, we simply default to
main
in PR builds, rather than adding Azure support.This means the URL will go to the wrong branch in docs previews for PRs against stable branches, although the stable branch build will work correctly. I don't expect this to matter much because PR builds are solely for docs previews; I'm skeptical code authors will be opening up the
[source]
link to double check the line numbers are exactly correct, since that line number calculation is 100% automated.This is an automatic backport of pull request #11851 done by Mergify.