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

Use sphinx.ext.linkcode for more precise source code links #11851

Merged
Merged
Show file tree
Hide file tree
Changes from 29 commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
099636d
switched from sphinx.ext.viewcode to sphinx.ext.linkcode
melechlapson Feb 20, 2024
00b05e7
removed extra line
melechlapson Feb 21, 2024
a20ad5c
Merge branch 'main' into mlapson/switch-to-sphinx-ext-linkcode
melechlapson Feb 21, 2024
2f94288
Add section header for source code links
melechlapson Feb 21, 2024
1d5c34d
removed docstring
melechlapson Feb 21, 2024
1849ce1
update return string
melechlapson Feb 21, 2024
cca9b04
added back blank line
melechlapson Feb 21, 2024
5bd18cd
Merge branch 'mlapson/switch-to-sphinx-ext-linkcode' of https://githu…
melechlapson Feb 21, 2024
b985ba6
Added a method to determine the GitHub branch
melechlapson Feb 21, 2024
914d2cd
Merge branch 'main' into mlapson/switch-to-sphinx-ext-linkcode
melechlapson Feb 21, 2024
1deb939
add blank line
melechlapson Feb 21, 2024
b84b399
remove print statement
melechlapson Feb 21, 2024
01e4ca6
Try to fix error for contextlib file
Eric-Arellano Feb 21, 2024
25ff4df
Try to fix error for Jenkins run #20240221.52
melechlapson Feb 21, 2024
ace069b
Check that qiskit in module name sooner
Eric-Arellano Feb 21, 2024
5420982
moved valid code object verification earlier
melechlapson Feb 27, 2024
d60bbf4
Merge branch 'main' into mlapson/switch-to-sphinx-ext-linkcode
melechlapson Feb 27, 2024
17881d4
added try except statement to getattr call
melechlapson Feb 27, 2024
9330e45
added extra try/except block
melechlapson Feb 27, 2024
ab8dad0
Also support Azure Pipelines
Eric-Arellano Feb 27, 2024
6ef1f68
removed unused import
melechlapson Feb 27, 2024
b45113b
Revert Azure support to keep things simple
Eric-Arellano Feb 27, 2024
a3e5297
added extra "/" to final URL
melechlapson Feb 27, 2024
e3382e7
Merge branch 'mlapson/switch-to-sphinx-ext-linkcode' of https://githu…
melechlapson Feb 27, 2024
825e02a
Merge branch 'main' into mlapson/switch-to-sphinx-ext-linkcode
melechlapson Feb 28, 2024
51f4bcb
Merge branch 'main' of github.com:Qiskit/qiskit-terra into mlapson/sw…
Eric-Arellano Mar 7, 2024
ab7f411
Move GitHub branch logic to GitHub Action
Eric-Arellano Mar 7, 2024
2a28edb
Merge branch 'mlapson/switch-to-sphinx-ext-linkcode' of github.com:me…
Eric-Arellano Mar 7, 2024
cde30ef
switched to importlib and removed redundant block of code
melechlapson Mar 20, 2024
5545bc8
Apply suggestions from code review
melechlapson Mar 21, 2024
7c95384
Merge branch 'main' into mlapson/switch-to-sphinx-ext-linkcode
melechlapson Mar 21, 2024
5b74a03
added back spaces
melechlapson Apr 4, 2024
8a805da
Merge branch 'main' of github.com:Qiskit/qiskit-terra into mlapson/sw…
Eric-Arellano Apr 4, 2024
abf4871
Clarify docs_deploy GitHub logic
Eric-Arellano Apr 4, 2024
3386c4c
Use pathlib for relativizing file name
Eric-Arellano Apr 4, 2024
614e8f8
Fix relative_to() path
Eric-Arellano Apr 5, 2024
19d052d
Remove tox prefix
Eric-Arellano Apr 5, 2024
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
17 changes: 14 additions & 3 deletions .github/workflows/docs_deploy.yml
Original file line number Diff line number Diff line change
Expand Up @@ -19,19 +19,30 @@ jobs:
with:
# We need to fetch the whole history so 'reno' can do its job and we can inspect tags.
fetch-depth: 0

- uses: actions/setup-python@v5
name: Install Python
with:
# Sync with 'documentationPythonVersion' in 'azure-pipelines.yml'.
python-version: '3.9'

- name: Install dependencies
melechlapson marked this conversation as resolved.
Show resolved Hide resolved
run: tools/install_ubuntu_docs_dependencies.sh

- name: Determine GitHub branch name
run: |
# PR workflows set the branch they're merging into.
if [ -n "$GITHUB_BASE_REF" ]; then
BRANCH_NAME="$GITHUB_BASE_REF"
# Tags like 1.0.0 and 1.0.0rc1 should point to their stable branch.
elif [[ $GITHUB_REF_NAME =~ ^([0-9]+\.[0-9]+) ]]; then
BRANCH_NAME="stable/${BASH_REMATCH[1]}"
else
BRANCH_NAME="$GITHUB_REF_NAME"
jakelishman marked this conversation as resolved.
Show resolved Hide resolved
Eric-Arellano marked this conversation as resolved.
Show resolved Hide resolved
fi
echo "Using branch '${BRANCH_NAME}' for GitHub source code links"
echo "QISKIT_DOCS_GITHUB_BRANCH_NAME=$BRANCH_NAME" >> $GITHUB_ENV

- name: Build documentation
run: tox run -e docs

- name: Store built documentation artifact
uses: actions/upload-artifact@v4
with:
Expand Down
48 changes: 46 additions & 2 deletions docs/conf.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,17 @@
# that they have been altered from the originals.

from __future__ import annotations
melechlapson marked this conversation as resolved.
Show resolved Hide resolved

# pylint: disable=invalid-name,missing-function-docstring

"""Sphinx documentation builder."""

melechlapson marked this conversation as resolved.
Show resolved Hide resolved
import datetime
import doctest
import importlib
import inspect
import os
import re


project = "Qiskit"
project_copyright = f"2017-{datetime.date.today().year}, Qiskit Development Team"
Expand All @@ -39,7 +43,7 @@
"sphinx.ext.intersphinx",
"sphinx.ext.doctest",
# This is used by qiskit/documentation to generate links to github.com.
"sphinx.ext.viewcode",
"sphinx.ext.linkcode",
"matplotlib.sphinxext.plot_directive",
"reno.sphinxext",
"sphinxcontrib.katex",
Expand Down Expand Up @@ -155,3 +159,43 @@
# ----------------------------------------------------------------------------------

plot_html_show_formats = False


# ----------------------------------------------------------------------------------
# Source code links
# ----------------------------------------------------------------------------------

def linkcode_resolve(domain, info):
if domain != "py":
return None

module_name = info["module"]
module = importlib.import_module(module_name)
if module is None or "qiskit" not in module_name:
return None
melechlapson marked this conversation as resolved.
Show resolved Hide resolved

obj = module
for part in info["fullname"].split("."):
try:
obj = getattr(obj, part)
except AttributeError:
return None

try:
full_file_name = inspect.getsourcefile(obj)
except TypeError:
return None
if full_file_name is None:
return None
file_name = full_file_name.split("/qiskit/")[-1]
Copy link
Member

Choose a reason for hiding this comment

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

It's quite unlikely to matter, but you might prefer to do this full_file_name.split thing using the proper os.path or pathlib tooling. Something like:

from pathlib import Path

REPO_ROOT = Path(__file__).resolve().parents[1]

def linkcode_resolve(domain, info):
    # ...

    file_name = str(Path(full_file_name).resolve().relative_to(REPO_ROOT / "qiskit"))

The current form would break if we were ever to add a subdirectory of the qiskit package that itself was called qiskit - unlikely, but not impossible.

Copy link
Member

Choose a reason for hiding this comment

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

Oh, actually not even unlikely - I just remembered that I literally proposed doing that in #10737 (though in that case, there wouldn't be any Python-documentable objects in it).

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks for the suggestion!

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah, I now remember why we were using [-1]. This current code results in the relative file path .tox/docs/lib/python3.8/site-packages/qiskit/circuit/classicalfunction/exceptions.py.

I'm unclear why that is: I couldn't reproduce in qiskit_sphinx_theme with setting in tox.ini isolated_build = true and usedevelop = false to mirror Qiskit. (I've been having issues running tox locally in Qiskit due to autodoc). In qiskit_sphinx_theme locally and GitHub Actions, the path is the normal file path you'd expect without the .tox prefix. So I think we want this code to support both styles.

We could use a regex replace to remove the .tox prefix if it's there.

Copy link
Member

Choose a reason for hiding this comment

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

In that case, perhaps you want to use import qiskit; qiskit.__file__ as the file-path root, so it'll automatically find the base of where the files are installed?

I don't mind a huge amount, though.


try:
source, lineno = inspect.getsourcelines(obj)
except (OSError, TypeError):
linespec = ""
else:
ending_lineno = lineno + len(source) - 1
linespec = f"#L{lineno}-L{ending_lineno}"

github_branch = os.environ.get("QISKIT_DOCS_GITHUB_BRANCH_NAME", "main")
return f"https://github.com/Qiskit/qiskit/tree/{github_branch}/qiskit/{file_name}{linespec}"
10 changes: 9 additions & 1 deletion tox.ini
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,15 @@ setenv =
QISKIT_SUPRESS_PACKAGING_WARNINGS=Y
QISKIT_TEST_CAPTURE_STREAMS=1
QISKIT_PARALLEL=FALSE
passenv = RAYON_NUM_THREADS, OMP_NUM_THREADS, QISKIT_PARALLEL, RUST_BACKTRACE, SETUPTOOLS_ENABLE_FEATURES, QISKIT_TESTS, QISKIT_IN_PARALLEL
passenv =
RAYON_NUM_THREADS
OMP_NUM_THREADS
QISKIT_PARALLEL
RUST_BACKTRACE
SETUPTOOLS_ENABLE_FEATURES
QISKIT_TESTS
QISKIT_IN_PARALLEL
QISKIT_DOCS_GITHUB_BRANCH_NAME
deps =
setuptools_rust # This is work around for the bug of tox 3 (see #8606 for more details.)
-r{toxinidir}/requirements.txt
Expand Down