Write complete manual `QuantumCircuit` documentation #12403

jakelishman · 2024-05-14T17:47:00Z

Summary

This writes huge tracts of new QuantumCircuit API documentation, linking together alike methods and writing explanatory text for how all the components fit together. There's likely an awful lot more that could go into this too, but this hopefully should impose a lot more order on the huge QuantumCircuit documentation page, and provide a lot more explanation for how the class works holistically.

In particular, the section on the control-flow builder interface could do with a lot more exposition and examples right now.

Details and comments

I ought to come back and write more about the control-flow builder interface with more examples, but this is already pushing it for 1.1 and I have other things that also need my attention right now.

@Eric-Arellano: at the moment, I've added QuantumCircuit to the toctree directly, especially because I don't want it to get generated with the standard templates we use for that. If that's going to be a problem / not great, we can look at alternatives.

This writes huge tracts of new `QuantumCircuit` API documentation, linking together alike methods and writing explanatory text for how all the components fit together. There's likely an awful lot more that could go into this too, but this hopefully should impose a lot more order on the huge `QuantumCircuit` documentation page, and provide a lot more explanation for how the class works holistically. In particular, the section on the control-flow builder interface could do with a lot more exposition and examples right now.

qiskit-bot · 2024-05-14T17:47:05Z

One or more of the the following people are requested to review this:

@Eric-Arellano
@javabster
@Qiskit/terra-core

coveralls · 2024-05-14T18:10:03Z

Pull Request Test Coverage Report for Build 9112130417

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

For more information on this, see Tracking coverage changes with pull request builds.
To avoid this issue with future PRs, see these Recommended CI Configurations.
For a quick fix, rebase this PR at GitHub. Your next report should be accurate.

Details

7 of 7 (100.0%) changed or added relevant lines in 1 file are covered.
44 unchanged lines in 6 files lost coverage.
Overall coverage decreased (-0.02%) to 89.6%

Files with Coverage Reduction	New Missed Lines	%
qiskit/passmanager/passmanager.py	1	92.63%
crates/qasm2/src/lex.rs	4	92.37%
crates/accelerate/src/euler_one_qubit_decomposer.rs	4	89.68%
qiskit/utils/parallel.py	10	82.76%
crates/qasm2/src/parse.rs	12	97.15%
crates/accelerate/src/sabre/neighbor_table.rs	13	73.13%

Totals
Change from base Build 9083265621:	-0.02%
Covered Lines:	62292
Relevant Lines:	69522

💛 - Coveralls

Eric-Arellano

Nice! Reviewers, check out https://qiskit-docs-preview-pr-1380.1799mxdls7qz.us-south.codeengine.appdomain.cloud/api/qiskit/dev/qiskit.circuit.QuantumCircuit to see how this renders.

--

@Eric-Arellano: at the moment, I've added QuantumCircuit to the toctree directly, especially because I don't want it to get generated with the standard templates we use for that. If that's going to be a problem / not great, we can look at alternatives.

I think it's good. FYI it results in the index page and ToC looking like this

I'm fine with the index page showing the class since it's so important. And I'm glad the left ToC is still sorted alphabetically.

Thanks for naming the file qiskit.circuit.QuantumCircuit so that we don't need redirects! It would be good to always use that naming scheme moving forward for modules too so that our URLs are more predictable: the import path.

Eric-Arellano · 2024-05-14T20:22:48Z

docs/apidoc/qiskit.circuit.QuantumCircuit.rst

+.. _qiskit-circuit-quantumcircuit:
+
+==============================
+:class:`.QuantumCircuit` class


Some notes if we stick with this rather than the simpler QuantumCircuit (no link):

We're not consistent with all the other class pages, e.g. https://docs.quantum.ibm.com/api/qiskit/qiskit.circuit.AncillaRegister

I don't think the link is very useful. On https://qiskit-docs-preview-pr-1380.1799mxdls7qz.us-south.codeengine.appdomain.cloud/api/qiskit/dev/qiskit.circuit.QuantumCircuit, it simply takes you to the class definition which is the first part of the page anyways

The styling is messed up for code in headers, tracked by https://github.ibm.com/IBM-Q-Software/iqp-channel-docs/issues/1037. If we stick with this approach, we'll try to fix the issue in the next week

Despite those concerns, I think I agree with your approach here, particularly because it makes the index page more readable with QuantumCircuit being escaped as code and having the word class. I tried to get a custom title to work in index.rst like '``QuantumCircuit`` class <qiskit.circuit.QuantumCircuit>', but RST doesn't like the inline formatting with `.

I mostly did this out of habit because I almost always put in cross-references when possible. If there's a way that works a little better, I can do that.

From my side, I don't mind the styling not being perfect immediately, if that's the remaining blocker (I'd seen it locally, too). It's a styling bug that we probably need to get fixed anyway, because I'd be surprised if it's not in use in at least some other places - I'm quite likely to have done it somewhere else in the API docs too - but it's still totally legible in the meantime, it just looks slightly odd.

If we could get the index.rst to properly render 'QuantumCircuit class <qiskit.circuit.QuantumCircuit>', then I would ask us to change to simply QuantumCircuit. But I couldn't get that working. So I think what you have is the best choice because it makes the index page more readable.

From my side, I don't mind the styling not being perfect immediately, if that's the remaining blocker

Indeed, it's my biggest concern. It's always been a problem though, and we'll bump https://github.ibm.com/IBM-Q-Software/iqp-channel-docs/issues/1037 up in priority. It's a short-term bug, so I agree it's not a blocker.

You mean a link where the inline code block and the word "class" are both the link text? If so: nested syntax (inline code inside a link) is (in)famously hard to do in rST. It's just about possible to hack together in some cases, but the syntax is at best arcane.

Sphinx-specific stuff is easier because that produces the nested elements by manipulating the docutils tree directly.

You mean a link where the inline code block and the word "class" are both the link text? If so: nested syntax (inline code inside a link) is (in)famously hard to do in rST. It's just about possible to hack together in some cases, but the syntax is at best arcane.

Yep, exactly. So, I think we should stick with how you have things.

This unblocks Qiskit/qiskit#12403. Without this PR, we incorrectly treat the class as inline, which means it gets the blue left bar and an h3 incorrectly added. Without this PR, our test to check that the ToC and index page are in sync also fails, even though things behave how we expect (Qiskit/qiskit#12403 (review))

jakelishman · 2024-05-15T12:55:48Z

Thanks for naming the file qiskit.circuit.QuantumCircuit so that we don't need redirects!

Oh, I missed this comment under the picture, but yeah no worries - I'm relatively sure I asked you and Arnau what I should name the file several weeks ago, so I'm just following instructions haha.

mtreinish

Overall this looks great, there is a lot much needed detail being added to the docs here which will be a huge help. I have left a few inline comments on some specific minor details. The only big thing I'm concerned about is how this ends up getting formatted on the hosted docs page. The way headings get set for the auto attributes makes it quite confusing, for example:

All the explainer text looks like it's in the wrong section because of this.

qiskit/circuit/quantumcircuit.py

Eric-Arellano · 2024-05-15T18:32:24Z

All the explainer text looks like it's in the wrong section because of this.

Fwiw, @arnaucasau and I realized this is because the margin-top for h3s is quite large. I started talking to one of the designers if we can reduce it. There's a chance we can reduce it a bit, but they aren't convinced yet we should.

Related context: we went back-and-forth on removing those auto-generated headers and decided to keep them. They're there to populate the right ToC and allow users to copy an anchor link.

So, the best improvement the docs app could make here is reducing margin-top for h3.

jakelishman · 2024-05-15T19:10:08Z

Eric: I'd say that the styling is actually correct here, the problem is that semantically, a function name etc shouldn't be a document heading. A heading implies that everything after it should be part of that title (which is what the CSS is currently set up to indicate), but that's not semantically what the documentation actually means; the function names are just things to be defined, and that doesn't impact the overall grouping structure of the document.

Sphinx's HTML output is semantically "definition lists" for these, which is (imo) more correct. I know that we currently need headers to make the right sidebar work, but I think that the most proper fix here is to fix the HTML output and then fix whatever knock-on effects that has.

Eric-Arellano · 2024-05-15T22:22:04Z

Eric: I'd say that the styling is actually correct here, the problem is that semantically, a function name etc shouldn't be a document heading.

You're spot on. I agree, and I appreciate you taking the time to think that through. The problem indeed becomes much more apparent with these custom-written module and class pages, compared to autosummary.

I opened Qiskit/documentation#1395. I still need to get designer sign-off, and it certainly won't be implemented by tomorrow's release, but I hope we can get to it within the next month. We're trying to finish this revamp of API docs before other priorities come up.

So, from the docs perspective, I wouldn't change anything about this PR re: the weird margins. The docs are written correctly and we'll try to fix this on the app side of things.

Co-authored-by: Matthew Treinish <[email protected]>

mtreinish

This LGTM now, thanks for the quick updates.

* Write complete manual `QuantumCircuit` documentation This writes huge tracts of new `QuantumCircuit` API documentation, linking together alike methods and writing explanatory text for how all the components fit together. There's likely an awful lot more that could go into this too, but this hopefully should impose a lot more order on the huge `QuantumCircuit` documentation page, and provide a lot more explanation for how the class works holistically. In particular, the section on the control-flow builder interface could do with a lot more exposition and examples right now. * Reword from review Co-authored-by: Matthew Treinish <[email protected]> * Add missing text around `Var` methods * Add example to `find_bit` * Comment on metadata through serialization * Add see-also sections to undesirable methods * Re-add internal utilities to documentation * Add examples to `depth` --------- Co-authored-by: Matthew Treinish <[email protected]> (cherry picked from commit 72ad545)

* Write complete manual `QuantumCircuit` documentation This writes huge tracts of new `QuantumCircuit` API documentation, linking together alike methods and writing explanatory text for how all the components fit together. There's likely an awful lot more that could go into this too, but this hopefully should impose a lot more order on the huge `QuantumCircuit` documentation page, and provide a lot more explanation for how the class works holistically. In particular, the section on the control-flow builder interface could do with a lot more exposition and examples right now. * Reword from review Co-authored-by: Matthew Treinish <[email protected]> * Add missing text around `Var` methods * Add example to `find_bit` * Comment on metadata through serialization * Add see-also sections to undesirable methods * Re-add internal utilities to documentation * Add examples to `depth` --------- Co-authored-by: Matthew Treinish <[email protected]> (cherry picked from commit 72ad545) Co-authored-by: Jake Lishman <[email protected]>

* Write complete manual `QuantumCircuit` documentation This writes huge tracts of new `QuantumCircuit` API documentation, linking together alike methods and writing explanatory text for how all the components fit together. There's likely an awful lot more that could go into this too, but this hopefully should impose a lot more order on the huge `QuantumCircuit` documentation page, and provide a lot more explanation for how the class works holistically. In particular, the section on the control-flow builder interface could do with a lot more exposition and examples right now. * Reword from review Co-authored-by: Matthew Treinish <[email protected]> * Add missing text around `Var` methods * Add example to `find_bit` * Comment on metadata through serialization * Add see-also sections to undesirable methods * Re-add internal utilities to documentation * Add examples to `depth` --------- Co-authored-by: Matthew Treinish <[email protected]>

This unblocks Qiskit/qiskit#12403. Without this PR, we incorrectly treat the class as inline, which means it gets the blue left bar and an h3 incorrectly added. Without this PR, our test to check that the ToC and index page are in sync also fails, even though things behave how we expect (Qiskit/qiskit#12403 (review))

jakelishman added this to the 1.1.0 milestone May 14, 2024

jakelishman requested a review from a team as a code owner May 14, 2024 17:47

Eric-Arellano reviewed May 14, 2024

View reviewed changes

Eric-Arellano mentioned this pull request May 14, 2024

Allow manually created class pages Qiskit/documentation#1382

Merged

mtreinish reviewed May 15, 2024

View reviewed changes

Eric-Arellano mentioned this pull request May 15, 2024

Move API headings inside the component Qiskit/documentation#1395

Open

jakelishman and others added 7 commits May 16, 2024 00:31

Reword from review

9564759

Co-authored-by: Matthew Treinish <[email protected]>

Add missing text around Var methods

bb5456e

Add example to find_bit

36b58ff

Comment on metadata through serialization

5cc79d7

Add see-also sections to undesirable methods

f6a2e50

Re-add internal utilities to documentation

36b53f4

Add examples to depth

32a7669

mtreinish approved these changes May 16, 2024

View reviewed changes

mtreinish added this pull request to the merge queue May 16, 2024

Merged via the queue into Qiskit:main with commit 72ad545 May 16, 2024
15 checks passed

jakelishman deleted the doc/quantumcircuit branch May 16, 2024 14:17

mergify bot mentioned this pull request May 16, 2024

Write complete manual QuantumCircuit documentation (backport #12403) #12420

Merged

jakelishman mentioned this pull request Sep 4, 2024

Documentation link anchors are not in the best place for some autoattributes #13085

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Write complete manual `QuantumCircuit` documentation #12403

Write complete manual `QuantumCircuit` documentation #12403

jakelishman commented May 14, 2024

qiskit-bot commented May 14, 2024

coveralls commented May 14, 2024 •

edited

Loading

Eric-Arellano left a comment •

edited

Loading

Eric-Arellano May 14, 2024

jakelishman May 14, 2024

Eric-Arellano May 14, 2024

jakelishman May 14, 2024

Eric-Arellano May 14, 2024

jakelishman commented May 15, 2024

mtreinish left a comment

Eric-Arellano commented May 15, 2024

jakelishman commented May 15, 2024

Eric-Arellano commented May 15, 2024

mtreinish left a comment

Write complete manual QuantumCircuit documentation #12403

Write complete manual QuantumCircuit documentation #12403

Conversation

jakelishman commented May 14, 2024

Summary

Details and comments

qiskit-bot commented May 14, 2024

coveralls commented May 14, 2024 • edited Loading

Pull Request Test Coverage Report for Build 9112130417

Warning: This coverage report may be inaccurate.

Details

💛 - Coveralls

Eric-Arellano left a comment • edited Loading

Choose a reason for hiding this comment

Eric-Arellano May 14, 2024

Choose a reason for hiding this comment

jakelishman May 14, 2024

Choose a reason for hiding this comment

Eric-Arellano May 14, 2024

Choose a reason for hiding this comment

jakelishman May 14, 2024

Choose a reason for hiding this comment

Eric-Arellano May 14, 2024

Choose a reason for hiding this comment

jakelishman commented May 15, 2024

mtreinish left a comment

Choose a reason for hiding this comment

Eric-Arellano commented May 15, 2024

jakelishman commented May 15, 2024

Eric-Arellano commented May 15, 2024

mtreinish left a comment

Choose a reason for hiding this comment

Write complete manual `QuantumCircuit` documentation #12403

Write complete manual `QuantumCircuit` documentation #12403

coveralls commented May 14, 2024 •

edited

Loading

Eric-Arellano left a comment •

edited

Loading