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 #2101

Merged
merged 11 commits into from
Apr 18, 2024

Conversation

melechlapson
Copy link
Contributor

@melechlapson melechlapson commented Apr 15, 2024

Summary

This is a follow-up to Qiskit/qiskit#11851 and 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:

a class definition https://github.com/Qiskit/qiskit-aer/blob/main/qiskit_aer/backends/aer_simulator.py#L40-L1046
a method https://github.com/Qiskit/qiskit-aer/blob/main/qiskit_aer/noise/noise_model.py#L849-L910
a function https://github.com/Qiskit/qiskit-aer/blob/main/qiskit_aer/utils/noise_transformation.py#L141-L232

Details and comments

Since some references in the API documentation actually refer to methods from the inherited qiskit classes, it was necessary to add a check at the end to change the base url if the method was from the qiskit class and link ot the qiskit repo.

Copy link
Contributor

@Eric-Arellano Eric-Arellano left a comment

Choose a reason for hiding this comment

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

Thanks!

docs/conf.py Outdated Show resolved Hide resolved
tox.ini Outdated Show resolved Hide resolved
docs/conf.py Outdated Show resolved Hide resolved
docs/conf.py Show resolved Hide resolved
Copy link
Contributor

@Eric-Arellano Eric-Arellano left a comment

Choose a reason for hiding this comment

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

Thanks! Let's double check the artifact is good in CI once that runs, but otherwise this looks good.

@Eric-Arellano
Copy link
Contributor

cc @chriseclectic and @doichanj (not sure if you watch the repo already)

@doichanj doichanj merged commit 2a448ba into Qiskit:main Apr 18, 2024
38 checks passed
@Eric-Arellano
Copy link
Contributor

FYI @doichanj we realized that the PR description and title were misleading - our bad! This change does not help with making things more precise - viewcode already took you to the precise source code. The main benefit of this PR is that it takes you to GitHub, rather than the Sphinx-generated copy of your code. The downside is more complex config.

See qiskit-community/qiskit-algorithms#170 (comment) for discussion.

I don't have a strong opinion. Let us know if you prefer to keep this or revert the change.

doichanj pushed a commit to doichanj/qiskit-aer that referenced this pull request Apr 25, 2024
* switched links to using sphinx.ext.linkcode

* added release note

* added check for qiskit methods

* remove whitespace

* removed regex subsitution

Co-authored-by: Eric Arellano <[email protected]>

* Update tox.ini

Co-authored-by: Eric Arellano <[email protected]>

* made updates following Eric's review

* made final line more readable

* cast filename to str

* re-added regex sub

---------

Co-authored-by: Eric Arellano <[email protected]>
@doichanj doichanj mentioned this pull request Apr 26, 2024
hhorii pushed a commit that referenced this pull request Apr 30, 2024
* Fix AerCompiler to use custom pass manager to decompose control flow ops (#2095)

* Fix AerCompiler to use basis_gates built from input circuit

* use custom pass manager to decompose dontrol flow ops

* remove unused import

* Use sphinx.ext.linkcode for more precise source code links (#2101)

* switched links to using sphinx.ext.linkcode

* added release note

* added check for qiskit methods

* remove whitespace

* removed regex subsitution

Co-authored-by: Eric Arellano <[email protected]>

* Update tox.ini

Co-authored-by: Eric Arellano <[email protected]>

* made updates following Eric's review

* made final line more readable

* cast filename to str

* re-added regex sub

---------

Co-authored-by: Eric Arellano <[email protected]>

* Fix noise sampling on shot-branching (#2098)

* Fix noise sampling on shot-branching

* format

* fix runtime noise sampling

* remove copying branch

* fix batch GPU

* format

* set initial value to Op structure

* format

* format

* test

* test

* fix use of additional_ops.size()

* fix error

* fix remove_empty_branches

* test

* test

* test

* test

* Fixes for dependency issues caused by 0.14 release (#2094)

* Fixes for dependency issues

* lint

* lint

* lint

* fix release note

* fix sampler

* fix sampler

* fix sampler

* fix sampler

* remove skip cp38

* hide primitives V2 for qiskit < 1.0

* lint

* add test case for sampling measure for large stabilizer circuit

* reduce warning

* replace test case for large stabilizer with GHZ circuit

* format

* format

* convert basis_gates from list to set

* fix assemble_circuits

* resolve conflicts

* Replace example in README to using primitives (#2105)

* Replace example in README to using primitives

* upgrade python version to 3.10 for github actions

* fix 3.10

* fix 3.10

* upgrade python version to 3.10 for github actions

* skip 3.8 and 3.9 for MacOS arm64

* skip 3.8 and 3.9 for MacOS arm64

* skip 3.8 and 3.9 for MacOS arm64

* replace macos-latest with macos-13

* Revert "Replace example in README to using primitives"

This reverts commit b536563.

* Revert "Revert "Replace example in README to using primitives""

This reverts commit 807ac6f.

* manually merge upstream

* add example using noise model

* remove print(result)

* add release note and set versions

* Fix CI failures (#2106)

* upgrade github actions for macos_arm64 and windows

* skip pp38/39

* exclude pp

* fix doc

* test

* remove CIBW_TEST_SKIP for wheel

---------

Co-authored-by: melechlapson <[email protected]>
Co-authored-by: Eric Arellano <[email protected]>
@Eric-Arellano
Copy link
Contributor

@doichanj Qiskit Algorithms ended up rejecting this change to keep things simple: qiskit-community/qiskit-algorithms#170 (comment). Would you like us to revert this PR or keep as is?

@doichanj
Copy link
Collaborator

doichanj commented May 7, 2024

I think it is OK to keep it as is

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants