-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Add Migration Guide for Opflow #9549
Conversation
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:
|
Pull Request Test Coverage Report for Build 4617374131
💛 - Coveralls |
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.
Thank you for great work! I left some comments.
|
||
.. code-block:: python | ||
|
||
from qiskit.opflow import PuliSumOp |
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 think Matthew said someway about running/testing these samples. Note sure if thats doctest - we have used that in apps. (Reason I comment here is due to typo in import - PuliSumOp
no a
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.
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 these are not tested at the moment, unless they are in a jupyter-execute environment 🙂
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.
So @woodsp-ibm, I managed to find an inbetween solution, I formatted the code examples to be runnable locally with pytest's doctest
. I don't know if I like having all of the >>>
in the code, but at least we can make sure that it runs.
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.
It is also possible to use ..testcode:: and ..testoutput:: directives (they are supported by the sphinx.ext.doctest, which we use in the apps and imagine would be used here too). By default the output puts these in separate highlighted blocks (though you can hide the testoutput if desired) but there is no need for >>> when using these.
I know though we used >>> in Nature etc since formatting with these former seemed harder to get things to come out right/as wanted etc. The >>> was always more concise and seemed easier to add and have it come out as expected in among other text. Maybe there is styling that can applied to testcode/output so things can be improved, not sure.
E.g.
.. testcode::
x = 1 + 3
print(x)
.. testoutput::
4
If this is published along with the API ref and by kept version, as the docs currently are, then this migration guide could be removed when opflow is removed. If not then afterwards the class refs will just be highlighted - just not links since they would not resolve. To me having them as class refs so they can be links seems better - since as links I can go to the opflow class itself for more detail if I need 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.
LGTM. Thanks a lot for working on this!
8c771ba
to
e162fca
Compare
cffadd9
to
8c771ba
Compare
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.
LGTM -- only one more left! 😄
* Add draft * Remove qiskit * Apply review * Apply comments boxnote * Small edit * Apply review * Update up to primitive ops * Dump changes from sphinx repo * Add twisties * Update information, add examples * Formatting changes * Simplify intro * Add links * Add sampler example * Update docs/migration_guides/opflow_migration.rst Co-authored-by: Steve Wood <[email protected]> * Update docs/migration_guides/opflow_migration.rst Co-authored-by: Julien Gacon <[email protected]> * Update docs/migration_guides/opflow_migration.rst Co-authored-by: Julien Gacon <[email protected]> * Update docs/migration_guides/opflow_migration.rst Co-authored-by: Julien Gacon <[email protected]> * Update docs/migration_guides/opflow_migration.rst Co-authored-by: Julien Gacon <[email protected]> * Apply suggestions from code review Co-authored-by: Julien Gacon <[email protected]> * Update CircuitOp Co-authored-by: Julien Gacon <[email protected]> * Applied suggestions locally * Change format, links * Add grouping, algorithms link * Fix indentation * Final review, add links * Add gradients * Apply heading suggestions Co-authored-by: Junye Huang <[email protected]> * Add tags * Apply suggestions code review * Correct code examples, add outputs * Run code examples by doctest * Remove underscores * Apply suggestions second round of review Co-authored-by: Junye Huang <[email protected]> * Add globals link * Change links to refs * Add dropdowns * Change doctest syntax, apply reviews * Try CI config * Add internal links * Fix lint, add doctest import * Fix cyclic import * Fix cyclic import * Apply suggestions from Declan's code review Co-authored-by: Declan Millar <[email protected]> * Update qiskit/quantum_info/__init__.py * Fix import * Copy docs config from Qiskit#9716 * Make black * Remove duplicate * Reorder --------- Co-authored-by: Steve Wood <[email protected]> Co-authored-by: Julien Gacon <[email protected]> Co-authored-by: Junye Huang <[email protected]> Co-authored-by: Declan Millar <[email protected]>
* Add draft * Remove qiskit * Apply review * Apply comments boxnote * Small edit * Apply review * Update up to primitive ops * Dump changes from sphinx repo * Add twisties * Update information, add examples * Formatting changes * Simplify intro * Add links * Add sampler example * Update docs/migration_guides/opflow_migration.rst Co-authored-by: Steve Wood <[email protected]> * Update docs/migration_guides/opflow_migration.rst Co-authored-by: Julien Gacon <[email protected]> * Update docs/migration_guides/opflow_migration.rst Co-authored-by: Julien Gacon <[email protected]> * Update docs/migration_guides/opflow_migration.rst Co-authored-by: Julien Gacon <[email protected]> * Update docs/migration_guides/opflow_migration.rst Co-authored-by: Julien Gacon <[email protected]> * Apply suggestions from code review Co-authored-by: Julien Gacon <[email protected]> * Update CircuitOp Co-authored-by: Julien Gacon <[email protected]> * Applied suggestions locally * Change format, links * Add grouping, algorithms link * Fix indentation * Final review, add links * Add gradients * Apply heading suggestions Co-authored-by: Junye Huang <[email protected]> * Add tags * Apply suggestions code review * Correct code examples, add outputs * Run code examples by doctest * Remove underscores * Apply suggestions second round of review Co-authored-by: Junye Huang <[email protected]> * Add globals link * Change links to refs * Add dropdowns * Change doctest syntax, apply reviews * Try CI config * Add internal links * Fix lint, add doctest import * Fix cyclic import * Fix cyclic import * Apply suggestions from Declan's code review Co-authored-by: Declan Millar <[email protected]> * Update qiskit/quantum_info/__init__.py * Fix import * Copy docs config from Qiskit#9716 * Make black * Remove duplicate * Reorder --------- Co-authored-by: Steve Wood <[email protected]> Co-authored-by: Julien Gacon <[email protected]> Co-authored-by: Junye Huang <[email protected]> Co-authored-by: Declan Millar <[email protected]>
* Add draft * Remove qiskit * Apply review * Apply comments boxnote * Small edit * Apply review * Update up to primitive ops * Dump changes from sphinx repo * Add twisties * Update information, add examples * Formatting changes * Simplify intro * Add links * Add sampler example * Update docs/migration_guides/opflow_migration.rst Co-authored-by: Steve Wood <[email protected]> * Update docs/migration_guides/opflow_migration.rst Co-authored-by: Julien Gacon <[email protected]> * Update docs/migration_guides/opflow_migration.rst Co-authored-by: Julien Gacon <[email protected]> * Update docs/migration_guides/opflow_migration.rst Co-authored-by: Julien Gacon <[email protected]> * Update docs/migration_guides/opflow_migration.rst Co-authored-by: Julien Gacon <[email protected]> * Apply suggestions from code review Co-authored-by: Julien Gacon <[email protected]> * Update CircuitOp Co-authored-by: Julien Gacon <[email protected]> * Applied suggestions locally * Change format, links * Add grouping, algorithms link * Fix indentation * Final review, add links * Add gradients * Apply heading suggestions Co-authored-by: Junye Huang <[email protected]> * Add tags * Apply suggestions code review * Correct code examples, add outputs * Run code examples by doctest * Remove underscores * Apply suggestions second round of review Co-authored-by: Junye Huang <[email protected]> * Add globals link * Change links to refs * Add dropdowns * Change doctest syntax, apply reviews * Try CI config * Add internal links * Fix lint, add doctest import * Fix cyclic import * Fix cyclic import * Apply suggestions from Declan's code review Co-authored-by: Declan Millar <[email protected]> * Update qiskit/quantum_info/__init__.py * Fix import * Copy docs config from Qiskit/qiskit#9716 * Make black * Remove duplicate * Reorder --------- Co-authored-by: Steve Wood <[email protected]> Co-authored-by: Julien Gacon <[email protected]> Co-authored-by: Junye Huang <[email protected]> Co-authored-by: Declan Millar <[email protected]>
* Add draft * Remove qiskit * Apply review * Apply comments boxnote * Small edit * Apply review * Update up to primitive ops * Dump changes from sphinx repo * Add twisties * Update information, add examples * Formatting changes * Simplify intro * Add links * Add sampler example * Update docs/migration_guides/opflow_migration.rst Co-authored-by: Steve Wood <[email protected]> * Update docs/migration_guides/opflow_migration.rst Co-authored-by: Julien Gacon <[email protected]> * Update docs/migration_guides/opflow_migration.rst Co-authored-by: Julien Gacon <[email protected]> * Update docs/migration_guides/opflow_migration.rst Co-authored-by: Julien Gacon <[email protected]> * Update docs/migration_guides/opflow_migration.rst Co-authored-by: Julien Gacon <[email protected]> * Apply suggestions from code review Co-authored-by: Julien Gacon <[email protected]> * Update CircuitOp Co-authored-by: Julien Gacon <[email protected]> * Applied suggestions locally * Change format, links * Add grouping, algorithms link * Fix indentation * Final review, add links * Add gradients * Apply heading suggestions Co-authored-by: Junye Huang <[email protected]> * Add tags * Apply suggestions code review * Correct code examples, add outputs * Run code examples by doctest * Remove underscores * Apply suggestions second round of review Co-authored-by: Junye Huang <[email protected]> * Add globals link * Change links to refs * Add dropdowns * Change doctest syntax, apply reviews * Try CI config * Add internal links * Fix lint, add doctest import * Fix cyclic import * Fix cyclic import * Apply suggestions from Declan's code review Co-authored-by: Declan Millar <[email protected]> * Update qiskit/quantum_info/__init__.py * Fix import * Copy docs config from Qiskit/qiskit#9716 * Make black * Remove duplicate * Reorder --------- Co-authored-by: Steve Wood <[email protected]> Co-authored-by: Julien Gacon <[email protected]> Co-authored-by: Junye Huang <[email protected]> Co-authored-by: Declan Millar <[email protected]>
Summary
This PR closes #9484
Blocked by #9716 (doctest & intersphinx config)Copied over docs config from #9716 (agreed upon with @HuangJunye)Details and comments
Open Questions:
TO-DOs:
StateFn
andexpectations
examplesevolutions
more concise