Skip to content
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

Remove deprecated classes and methods in quantum_info #8070

Merged
merged 37 commits into from
Jun 23, 2022

Conversation

daimurat
Copy link
Member

@daimurat daimurat commented May 16, 2022

Summary

This is a part of QAMP #20

@daimurat daimurat requested review from a team, ikkoham, manoelmarques and woodsp-ibm as code owners May 16, 2022 15:21
@qiskit-bot
Copy link
Collaborator

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:

@coveralls
Copy link

coveralls commented May 16, 2022

Pull Request Test Coverage Report for Build 2548390936

  • 17 of 23 (73.91%) changed or added relevant lines in 7 files are covered.
  • 4 unchanged lines in 2 files lost coverage.
  • Overall coverage decreased (-0.03%) to 83.915%

Changes Missing Coverage Covered Lines Changed/Added Lines %
qiskit/quantum_info/operators/symplectic/pauli.py 1 7 14.29%
Files with Coverage Reduction New Missed Lines %
qiskit/quantum_info/operators/symplectic/base_pauli.py 1 87.13%
qiskit/quantum_info/operators/symplectic/pauli.py 3 80.23%
Totals Coverage Status
Change from base Build 2546341916: -0.03%
Covered Lines: 55374
Relevant Lines: 65988

💛 - Coveralls

@ikkoham ikkoham added Changelog: Removal Include in the Removed section of the changelog mod: quantum info Related to the Quantum Info module (States & Operators) QAMP issues and PRs relating to Qiskit Advocates Mentorship Program labels May 17, 2022
@ikkoham
Copy link
Contributor

ikkoham commented May 17, 2022

I noticed that the deprecated path is used in Aer (Qiskit/qiskit-aer#1521). So we can't merge this PR until Aer is released.

@ikkoham ikkoham added the on hold Can not fix yet label May 17, 2022
@HuangJunye
Copy link
Collaborator

@daiki0623 @ikkoham Is this part of QAMP? If so, can you please link to the project issue in the description of the PR? I will also add the QAMP label to the PR. Thank you!

@daimurat
Copy link
Member Author

daimurat commented May 18, 2022

@daiki0623 @ikkoham Is this part of QAMP? If so, can you please link to the project issue in the description of the PR? I will also add the QAMP label to the PR. Thank you!

Thank you, added QAMP link in the description.

@mtreinish
Copy link
Member

mtreinish commented Jun 22, 2022

This is failing CI because the operators tutorial is using Pauli(label="XX") (or similar) a bunch of places. However, I think this is pointing to a bug in the deprecation prior to this since there are no warnings being raised in the tutorial. For example see the rendered hosted version at: https://qiskit.org/documentation/tutorials/circuits_advanced/02_operators_overview.html#Converting-classes-to-Operators

I push Qiskit/qiskit-tutorials#1339 to update the tutorials, but maybe we want to hold off for a cycle on this and ensure we're actually emitting proper warnings before we remove it.

mtreinish added a commit to mtreinish/qiskit-tutorial that referenced this pull request Jun 22, 2022
This commit updates the operator tutorial to avoid the deprecated Pauli
constructor kwarg, label. This has been deprecated since Qiskit Terra
0.17.0 and is pending removal in Qiskit/qiskit#8070. The label
kwarg isn't needed anymore as the Pauli string can just be input
directly as the first positional argument and the Pauli object will be
created just as with the label kwarg before.
@jakelishman
Copy link
Member

That's on me - the PR had a deprecate_arguments({"label": "data"}) decorator on the initialiser before, which I removed as unnecessary because the deprecation had been active before, and removing it didn't affect local tests.

I bet the issue is that the stack level deprecate_function sets it blame the caller of the function directly, but the function in question here is our __init__, so the error won't appear by default.

If we reinstate the deprecation on the initialiser for this cycle, that's probably best.

@jakelishman
Copy link
Member

jakelishman commented Jun 22, 2022

@mtreinish: I've reinstated the constructors whose deprecations failed to show, and left everything else removed.

Copy link
Contributor

@ikkoham ikkoham left a comment

Choose a reason for hiding this comment

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

LGTM. I hadn't thought of a way to use __getitem__ to generate DeprecationWarning. Thanks.
I'll make it a comment so it doesn't get automerged.

Copy link
Member

@mtreinish mtreinish left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for the quick update restoring and fixing the deprecation warnings.

@mergify mergify bot merged commit 8455c9f into Qiskit:main Jun 23, 2022
SooluThomas pushed a commit to Qiskit/qiskit-tutorials that referenced this pull request Jun 23, 2022
This commit updates the operator tutorial to avoid the deprecated Pauli
constructor kwarg, label. This has been deprecated since Qiskit Terra
0.17.0 and is pending removal in Qiskit/qiskit#8070. The label
kwarg isn't needed anymore as the Pauli string can just be input
directly as the first positional argument and the Pauli object will be
created just as with the label kwarg before.
ElePT pushed a commit to ElePT/qiskit-tutorials that referenced this pull request Nov 23, 2022
…it#1339)

This commit updates the operator tutorial to avoid the deprecated Pauli
constructor kwarg, label. This has been deprecated since Qiskit Terra
0.17.0 and is pending removal in Qiskit/qiskit#8070. The label
kwarg isn't needed anymore as the Pauli string can just be input
directly as the first positional argument and the Pauli object will be
created just as with the label kwarg before.
Eric-Arellano pushed a commit to Eric-Arellano/qiskit-terra that referenced this pull request Aug 25, 2023
…it/qiskit-tutorials#1339)

This commit updates the operator tutorial to avoid the deprecated Pauli
constructor kwarg, label. This has been deprecated since Qiskit Terra
0.17.0 and is pending removal in Qiskit#8070. The label
kwarg isn't needed anymore as the Pauli string can just be input
directly as the first positional argument and the Pauli object will be
created just as with the label kwarg before.
Eric-Arellano pushed a commit to Eric-Arellano/qiskit-terra that referenced this pull request Sep 8, 2023
…it/qiskit-tutorials#1339)

This commit updates the operator tutorial to avoid the deprecated Pauli
constructor kwarg, label. This has been deprecated since Qiskit Terra
0.17.0 and is pending removal in Qiskit#8070. The label
kwarg isn't needed anymore as the Pauli string can just be input
directly as the first positional argument and the Pauli object will be
created just as with the label kwarg before.
Eric-Arellano pushed a commit to Qiskit/documentation that referenced this pull request Oct 6, 2023
…it/qiskit-tutorials#1339)

This commit updates the operator tutorial to avoid the deprecated Pauli
constructor kwarg, label. This has been deprecated since Qiskit Terra
0.17.0 and is pending removal in Qiskit/qiskit#8070. The label
kwarg isn't needed anymore as the Pauli string can just be input
directly as the first positional argument and the Pauli object will be
created just as with the label kwarg before.
Eric-Arellano pushed a commit to Qiskit/documentation that referenced this pull request Oct 9, 2023
…it/qiskit-tutorials#1339)

This commit updates the operator tutorial to avoid the deprecated Pauli
constructor kwarg, label. This has been deprecated since Qiskit Terra
0.17.0 and is pending removal in Qiskit/qiskit#8070. The label
kwarg isn't needed anymore as the Pauli string can just be input
directly as the first positional argument and the Pauli object will be
created just as with the label kwarg before.
Eric-Arellano pushed a commit to Qiskit/documentation that referenced this pull request Oct 9, 2023
…it/qiskit-tutorials#1339)

This commit updates the operator tutorial to avoid the deprecated Pauli
constructor kwarg, label. This has been deprecated since Qiskit Terra
0.17.0 and is pending removal in Qiskit/qiskit#8070. The label
kwarg isn't needed anymore as the Pauli string can just be input
directly as the first positional argument and the Pauli object will be
created just as with the label kwarg before.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changelog: Removal Include in the Removed section of the changelog Community PR PRs from contributors that are not 'members' of the Qiskit repo mod: quantum info Related to the Quantum Info module (States & Operators) QAMP issues and PRs relating to Qiskit Advocates Mentorship Program
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

7 participants