-
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
Apply new deprecation decorators to circuit folder #9869
Apply new deprecation decorators to circuit folder #9869
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: |
…ply-deprecations-circuit
Pull Request Test Coverage Report for Build 4865768041
💛 - Coveralls |
This reverts commit 54f6ee8.
…ply-deprecations-circuit
Bump |
…ply-deprecations-circuit
# Over-specific import to avoid cyclic imports. | ||
from qiskit.utils.deprecation import deprecate_function | ||
from qiskit.utils.deprecation import deprecate_func |
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 mean, the comment is still true lol. It's fine to get rid of it, though.
qiskit/circuit/instructionset.py
Outdated
"(The classical registers are insufficient to access all classical resources in a " | ||
"circuit, as there may be loose classical bits as well. It can also cause integer " | ||
"indices to be resolved incorrectly if any registers overlap.)" |
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.
super nitpicky, but the parenthetical sentences here end up being longer than the actual deprecation message, which feels pretty weird to me. It's either important information for the user, in which case it needn't be parenthesised, or it's not and it can be a code/docs-only comment instead. I don't particularly mind which.
r"'Bit\.(register|index)' is deprecated.*", | ||
r"The property ``qiskit\.circuit\.bit\.Bit\.(register|index)`` is deprecated.*", |
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.
this is only semi-related: the error message we've got here shows a potential place for improvement in the decorators to me; qiskit.circuit.bit.Bit.register
feels way too long to put out. I think Bit.register
would be less noisy - the user will always be able to inspect the object themselves if they need to know more about it.
…ply-deprecations-circuit
…ply-deprecations-circuit
* Apply new deprecation decorators to circuit folder * Fix what I can (blocked by production issues) * Work around tests using bad production code * Revert "Work around tests using bad production code" This reverts commit 54f6ee8. * Better fix for the deprecations * Use QiskitTestCase ignore mechanism * Review feedback * Fix lint and test failure (cherry picked from commit 5323d8f)
* Apply new deprecation decorators to circuit folder * Fix what I can (blocked by production issues) * Work around tests using bad production code * Revert "Work around tests using bad production code" This reverts commit 54f6ee8. * Better fix for the deprecations * Use QiskitTestCase ignore mechanism * Review feedback * Fix lint and test failure (cherry picked from commit 5323d8f) Co-authored-by: Eric Arellano <[email protected]>
* Apply new deprecation decorators to circuit folder * Fix what I can (blocked by production issues) * Work around tests using bad production code * Revert "Work around tests using bad production code" This reverts commit 54f6ee8. * Better fix for the deprecations * Use QiskitTestCase ignore mechanism * Review feedback * Fix lint and test failure
Summary
See #9676 for the motivation.
Details and comments