Skip to content

Add exceptions to API documentation#10522

Merged
mtreinish merged 2 commits into
Qiskit:mainfrom
jakelishman:document-exceptions
Aug 14, 2023
Merged

Add exceptions to API documentation#10522
mtreinish merged 2 commits into
Qiskit:mainfrom
jakelishman:document-exceptions

Conversation

@jakelishman
Copy link
Copy Markdown
Member

Summary

The big change here is that QiskitError is added to the API documentation, which causes 400--500 previously failing Sphinx lookups (stemming from "Raises:" documentation) to now succeed. This commit also corrects several other places where exceptions were not being fully documented (and in several cases makes the imports more convenient as well), and corrects a couple of places with incorrect references to exceptions.

Details and comments

This stemmed from an idle thought I had to turn on nitpicky in the Sphinx configuration, which emits a warning if any cross-reference fails to resolve. That's probably a longer-term effort: including the ones fixed by this PR, there were 2791 warnings. It might cause some more nuisances with release notes and removals, but we'll have to see how that goes if we decide to actually turn the option on. The principle of making sure that most of our references resolve is good regardless, though.

qiskit.algorithms and qiskit.opflow are both deprecated but emit sizeable numbers of references, and the strategy here is probably just going to be to leave them be, since they're scheduled for removal from this repo anyway.

@jakelishman jakelishman added documentation Something is not clear or an error documentation stable backport potential Make Mergify open a backport PR to the most recent stable branch on merge. Changelog: None Do not include in the GitHub Release changelog. labels Jul 28, 2023
@qiskit-bot
Copy link
Copy Markdown
Collaborator

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

@jakelishman jakelishman force-pushed the document-exceptions branch from ccbc524 to bf42192 Compare July 28, 2023 11:14
The (1 - alpha) confidence interval of the specified kind.

Raises:
AquaError: If 'mle' is not in self._ret.keys() (i.e. `run` was not called yet).
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Obviously AquaError is wrong here, but as best as I could tell it was already out-of-date when it was merged in #5517, which seems to have involved some refactoring from old Aqua code that I couldn't be bothered to track down.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

:) I happened to notice this too in qiskit-algorithms as that was prepared for release and it was taken care of there. I figured it had been here for years like this since it was brought over from Aqua and never noticed, and as it was not causing an issue and with algorithms set to be removed I.... (was the same "fix" we did there!)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Ah, that's good that you'd spotted it on the new repo as well. I had a go at tracking down a point where the result object might have actually raised an error if mle wasn't present, but once it got into needing to find the base of refactored history from Aqua, I just gave up...

Copy link
Copy Markdown
Collaborator

@Cryoris Cryoris Aug 14, 2023

Choose a reason for hiding this comment

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

This is from an old (very old) version of the code, where the algorithm classes were still stateful. Back then you had to run the algorithm to generate some info and then this enabled some post processing options.

The big change here is that `QiskitError` is added to the API
documentation, which causes 400--500 previously failing Sphinx lookups
(stemming from "Raises:" documentation) to now succeed.  This commit
also corrects several other places where exceptions were not being fully
documented (and in several cases makes the imports more convenient as
well), and corrects a couple of places with incorrect references to
exceptions.
@jakelishman jakelishman force-pushed the document-exceptions branch from bf42192 to a2fdddb Compare July 28, 2023 11:40
@mtreinish mtreinish added this to the 0.25.1 milestone Jul 28, 2023
@coveralls
Copy link
Copy Markdown

coveralls commented Jul 28, 2023

Pull Request Test Coverage Report for Build 5837187759

  • 8 of 8 (100.0%) changed or added relevant lines in 8 files are covered.
  • 4 unchanged lines in 1 file lost coverage.
  • Overall coverage increased (+0.006%) to 87.262%

Files with Coverage Reduction New Missed Lines %
crates/qasm2/src/lex.rs 4 90.63%
Totals Coverage Status
Change from base Build 5836645682: 0.006%
Covered Lines: 74293
Relevant Lines: 85138

💛 - Coveralls

Copy link
Copy Markdown
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.

This looks great! Thanks Jake.

@mtreinish mtreinish added this pull request to the merge queue Aug 14, 2023
Merged via the queue into Qiskit:main with commit 982807e Aug 14, 2023
mergify Bot pushed a commit that referenced this pull request Aug 14, 2023
The big change here is that `QiskitError` is added to the API
documentation, which causes 400--500 previously failing Sphinx lookups
(stemming from "Raises:" documentation) to now succeed.  This commit
also corrects several other places where exceptions were not being fully
documented (and in several cases makes the imports more convenient as
well), and corrects a couple of places with incorrect references to
exceptions.

(cherry picked from commit 982807e)

# Conflicts:
#	docs/apidocs/exceptions.rst
#	qiskit/dagcircuit/__init__.py
#	qiskit/passmanager/__init__.py
#	qiskit/transpiler/__init__.py
jaeunkim pushed a commit to jaeunkim/qiskit-terra that referenced this pull request Aug 15, 2023
The big change here is that `QiskitError` is added to the API
documentation, which causes 400--500 previously failing Sphinx lookups
(stemming from "Raises:" documentation) to now succeed.  This commit
also corrects several other places where exceptions were not being fully
documented (and in several cases makes the imports more convenient as
well), and corrects a couple of places with incorrect references to
exceptions.
github-merge-queue Bot pushed a commit that referenced this pull request Aug 15, 2023
* Add exceptions to API documentation (#10522)

The big change here is that `QiskitError` is added to the API
documentation, which causes 400--500 previously failing Sphinx lookups
(stemming from "Raises:" documentation) to now succeed.  This commit
also corrects several other places where exceptions were not being fully
documented (and in several cases makes the imports more convenient as
well), and corrects a couple of places with incorrect references to
exceptions.

(cherry picked from commit 982807e)

# Conflicts:
#	docs/apidocs/exceptions.rst
#	qiskit/dagcircuit/__init__.py
#	qiskit/passmanager/__init__.py
#	qiskit/transpiler/__init__.py

* Fix merge conflicts

This commit fixes several merge conflicts in the backport. They were
mostly minor differences between main and the stable branch that needed
to be corrected.

---------

Co-authored-by: Jake Lishman <jake.lishman@ibm.com>
Co-authored-by: Matthew Treinish <mtreinish@kortar.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Changelog: None Do not include in the GitHub Release changelog. documentation Something is not clear or an error documentation stable backport potential Make Mergify open a backport PR to the most recent stable branch on merge.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants