Remove dependency on sphinx-autodoc-typehints

[jakelishman]

Summary

This replaces the whole usage of the separate package sphinx-autodoc-typehints with the built-in support from autodoc, available (with this combination of options) since Sphinx 5.0. This removes a docs dependency that has caused us significant problems before due to its instability, and the built-in handling in autodoc is much cleaner as well; rather than trying to textually modify the docstring, this version (correctly) mutates the parsed rST structure, which also naturally it 100% compatible with the Napoleon processing.

Details and comments

I set the type-hint location to be both in the signatures and descriptions (our current behaviour is only in the descriptions, unless there's a typing.overload). I'm happy to make it either just signatures or just descriptions, as people prefer. Here's an example of the new output for QuantumCircuit.if_else (a function that currently only has type hints in the signature):

(The slightly buggy output is because of Qiskit/qiskit_sphinx_theme#183; it's pre-existing for this change.)

For reference, here's what it looks like in the built documentation.

Thanks to @levbishop for pointing out that autodoc does this natively now.

[qiskit-bot]

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:

• @Qiskit/terra-core

[coveralls]

Pull Request Test Coverage Report for Build 4316872058

• 0 of 0 changed or added relevant lines in 0 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage remained the same at 85.343%

---

Totals
Change from base Build 4309626206:	0.0%
Covered Lines:	67530
Relevant Lines:	79128

---

💛 - Coveralls

[woodsp-ibm]

Having the typehints in the signature as well make it too busy for me personally and hard to read it, as I ave to search hard to try find the parameter names, which gives me a clue what it takes. I much prefer just the names/default values of the parameters in the signature, as before, which for me gives a better overview. And if I need the type and more detail on what it does I can look at parameters.

[jakelishman]

We can do that instead if people prefer - it's a one-line configuration change. If we're on board with this PR in general (I'll wait for technical review), I'll post a little vote to gather people's preferences, and at the end of the day, the docs squad can say how they prefer the information is presented.

This particular example is more complex because its allowed inputs are very permissive - in the actual code they're hidden behind a type alias (QubitSpecifier), but Sphinx at the moment is resolving that into the concrete output.

[woodsp-ibm]

Sure, I am just mindful of the fact we use typehinting extensively in the algos and apps (and we would want to change the apps docs to whatever is done for consistency). VQE for instance goes from

to

BTW I was hoping it might improve the output for dataclasses where the types in the attributes are all capitalized - sadly it did not from what I looked at.

[jakelishman]

types in the attributes are all capitalized

I'd be very surprised if Sphinx itself is modifying the HTML output to capitalise everything. Do you have a link to an HTML page where that's happening? Guessing completely in the dark, it sounds more like a wayward CSS text-transform: uppercase rule in the theme than something Sphinx would be doing.

[jakelishman]

By the way: the main point of this PR isn't meant to be a change in the style, it's the change to do the type-hint interpretation using Sphinx built-ins rather than the flaky sphinx-autodoc-typehints package. I'm on board with whichever style people want the PR to merge with.

[woodsp-ibm]

Yeah, I understood the main point was to change to the built in way.

A dataclass example https://qiskit.org/documentation/stubs/qiskit.algorithms.optimizers.OptimizerState.html I recall other formatting weirdness - not sure but I seem to recall if there was an underscore in the name.

Update: Yes does seem to be related to an underscore, you can see that for quasi_dicts attribute in the SamplerResult dataclass https://qiskit.org/documentation/stubs/qiskit.primitives.SamplerResult.html

[jakelishman]

Yeah - it's rogue CSS rather than rogue Sphinx output:

The HTML there has (e.g.) <span class="pre">Optional</span>, and that CSS rule in the image above is incorrectly affecting it.

[jakelishman]

(Upper-casing issue filed as Qiskit/qiskit_sphinx_theme#194.)

[Eric-Arellano]

Excellent! I vaguely prefer the type hints to be in both, but I don't have a strong opinion.

I'm fine with sticking with the status quo so that this PR indeed only changes our build process, not the format. No need to couple those two.

[HuangJunye]

I haven't looked at the changes yet but I just want to point out that the meta doc currently still builds with Sphinx 4.5 so that is a potential blocker. I am whether there is something preventing us to use higher versions or simply because we pinned it to ~4.0. https://github.com/Qiskit/qiskit/blob/0163282e497391910cd091ffd7c1d55dc0b5d880/requirements-dev.txt#L2

[jakelishman]

@HuangJunye: I think that pin's out of date. It was added in #1470, which if I remember correctly was after we finally managed to navigate the 3.0->4.0 transition that had been very hard for us. Since then though, Sphinx major versions have been much less breaking changes so the defensive pin likely isn't necessary. Terra's docs are built with Sphinx 5 (I think there's a transitive pin preventing Sphinx 6 and 7 from being used), and as far as I know, there's nothing in Aer or ibmq-provider that would require the lower build.

[woodsp-ibm]

Yeah sphinx-design is not compatible with Sphinx 6 - I tried updating before and at least that forced things back to 5.

[jakelishman]

Ok, 9a92f3a should have set the type-hint configuration to be just about the same as the status quo. I'll leave it to somebody else in another PR if they want to cast a poll on what the docs style should be.

[Eric-Arellano]

For this to impact our final docs we deploy, I think this needs to update qiskit-metapackage too, right?

My understanding of our setup is that qiskit-metapackage only clones Terra's docs/apidocs folder:

https://github.com/Qiskit/qiskit-metapackage/blob/20783d7c4d6a3622e2c24cfa94656d008723a5f5/docs/custom_extensions.py#L66-L84

https://github.com/Qiskit/qiskit-metapackage/blob/20783d7c4d6a3622e2c24cfa94656d008723a5f5/docs/custom_extensions.py#L57-L59

We don't use Terra's conf.py to then build those copied docs. Is that right?

If so, I'm happy to make this update to qiskit-metapackage.

[jakelishman]

Oh yeah thanks, I meant to copy this to the metapackage once this had merged, but forgot. Done in Qiskit/qiskit-metapackage#1716.