This repository has been archived by the owner on Aug 19, 2023. It is now read-only.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fix most Sphinx warnings #1737
Fix most Sphinx warnings #1737
Changes from 8 commits
059eb65
bd988b7
51adbd0
285b46a
38a9595
45a3e83
72d9bff
ac1604f
7d3d0e0
3d1558a
e3fd4d4
5d73f90
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't pin this in Terra. Seemed unnecessary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm pretty sure we did right when jinja2 3.1 was released, but we subsequently released the pin on Terra and forgot to do so on the metapackage.
edit: yeah, Qiskit/qiskit#7815 added it, and Qiskit/qiskit#8968 removed it again.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Forgot to delete this in a prior PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are all copied from qiskit-tutorials.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not important, but just for your writing convenience if you like in the future: you can combine the
~
(strip leading prefixes) and.
(suffix lookup) operators in Sphinx at whatever level of prefix you like. In:class:`~.Z2Symmetries`
the~
is redundant because there's no prefix to strip, but:class:`~.quantum_info.Z2Symmetries`
would usequantum_info.Z2Symmetries
as the suffix to lookup (and so be disambiguated), then~
would causequantum_info
to be stripped in the output display.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fyi @jakelishman these still lose cross references because I could not find the file in Terra. It's the ones that are
notes/<foo>.yaml
, whereas I could find the files fornotes/<version>/<foo>.yaml
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That probably happens because we move the files in Terra from being in the root
releasenotes/notes
directory to a versioned directory as part of the minor-release procedure, but we don't tend to bother when doing patch releases on the stable branch; they only get moved later when we silo off the next minor release. We wouldn't bother going through the comments in the metapackage updating stuff.I'm not sure why that causes us to lose the cross-references. Fwiw, that particular note will be referring to the Opflow version of
Z2Symmetries
- it's from before thequantum_info
version was added - so while it's still a valid cross-reference for now, it won't be once Opflow is removed in a couple of versions' time.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, I realize this wasn't clear. These cross-references are ambiguous so Sphinx has a warning it can't link properly.
The only way in other places I was able to disambiguate is by looking up in Terra what the updated release notes are. Those were already disambiguate in Terra, just not updated here.
For here and the few other references I had to remove, I both could not find the file name with
fd
nor some sample text from the release note, e.g. neither of these match this note:Rather than trying to guess how to disambiguate, I removed the cross-reference.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, it's fair enough - I generally suggest that if the natural Sphinx references cause Sphinx to complain about ambiguity then there's a very high chance that the reader will be confused as well, so it's worth including more lead-in to the link (e.g.
:meth:`.opflow.Z2Symmetries.taper`
). That said, these are old and their targets are going to disappear in the not-too-distant future, so it's fine to just drop them now.I'm not sure what
fd
program you're referring to, but the search fails because the release note was rewritten during its backporting (see Qiskit/qiskit#8410 and Qiskit/qiskit#8514) - looks like the note got merged tomain
in a not-great state, and Matthew rewrote it before it'd appear. Since patch-release notes are just hidden onmain
as part of the minor-release procedure, we don't always bother to bring any changes in.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fd
is an amazing rewrite offind
. Similar torg
. See https://github.com/unpluggedcoder/awesome-rust-toolsThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Forgot to delete this in a prior PR.