-
Notifications
You must be signed in to change notification settings - Fork 90
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
API generation script: fix attributes headings #1153
Conversation
Thanks for contributing to Qiskit documentation! 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. Thanks! 🙌 |
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.
Thanks! This is a net improvement but I noticed a couple of quirks
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.
Nice work, thanks!
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.
Thanks! Can you please clarify in the PR that you regenerated the HTML to fix the bad attribute? This PR is a mix of using the new HTML + your new code change
@@ -172,7 +172,7 @@ These functions will raise a custom subclass of [`QiskitError`](exceptions#qiski | |||
Set the error message. | |||
</Class> | |||
|
|||
### QPY\_VERSION | |||
### qiskit.qpy.QPY\_VERSION |
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.
Do you know what these were before the docs redesign? I'm not certain it makes sense to use the full path here because it makes the header longer. I'm surprised the name is the full path and not only QPY_VERSION
. It's not necessarily bad, I'm only curious why name
was set to that
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.
Yes, they had the long name
as well. Here is the file before the docs redesign. I can investigate more about why it was set that way if you want :)
Closes Qiskit#1120 This PR changes the script to use the `name` extracted from the `signature` of the attributes instead of using the `id` to create the `<h3>`s. This behavior was changed in Qiskit#1026, but It broke a couple of headings. It also fixes two attributes of `qiskit.algorithms.eigensolvers.VQD` manually modifying the artifacts stored in [Box](https://ibm.ent.box.com/folder/246867452622?s=tqmvw6ldivzkhh934nhufan4rg3dregr) for versions 0.39, 0.40, 0.41, 0.42, 0.43, 0.44, 0.45 and 0.46. See more at Qiskit#1120
Closes #1120
This PR changes the script to use the
name
extracted from thesignature
of the attributes instead of using theid
to create the<h3>
s. This behavior was changed in #1026, but It broke a couple of headings.It also fixes two attributes of
qiskit.algorithms.eigensolvers.VQD
manually modifying the artifacts stored in Box for versions 0.39, 0.40, 0.41, 0.42, 0.43, 0.44, 0.45 and 0.46. See more at #1120