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

[wip] Better handle pending deprecations #9601

Closed
wants to merge 3 commits into from

Conversation

Eric-Arellano
Copy link
Collaborator

Summary

Our deprecation policy distinguishes between DeprecationWarning vs PendingDeprecationWarning: https://github.com/Qiskit/qiskit/blob/82810146c5061fad17c041e7e66196f92466ec1a/docs/deprecation_policy.rst#removing-a-feature

This PR improves support for PendingDeprecationWarning:

  1. Fixes wording for deprecate_arguments to clarify when something is pending.
  2. Makes the interface for marking something as pending clearer.
    • Before, we had you pass in category: type, which was unbounded to be the whole universe of Python types. When in reality, we only expected it to be either DeprecationWarning or PendingDeprecationWarning.
    • Bounding to only pending={False,True} makes clear this reality. It will help in future changes (like Deprecate warning box in docs #8600) to conditionally handle pending vs deprecated.

Details and comments

This is prework for the rework ofchttps://github.com//pull/8600. I want to make sure our docs properly support PendingDeprecationWarning.

@Eric-Arellano Eric-Arellano added Changelog: None Do not include in changelog refactor Proposal to refactor code labels Feb 16, 2023
@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:

@Eric-Arellano
Copy link
Collaborator Author

Hm, looks like deprecation.py is used by repos outside of Terra, like

https://github.com/Qiskit/qiskit-machine-learning/blob/89eac3b39924ebfdcbb2b05a22d70e745e791c09/qiskit_machine_learning/neural_networks/circuit_qnn.py#L51-L58

So, the pending: bool change is not safe. It would need to be done via a deprecation, which has some overhead.

This PR was the first of several improvements I was hoping to make in service of #8600. I'll mark this as WIP and regroup. Sorry for the noise!

@Eric-Arellano Eric-Arellano changed the title Better handle pending deprecations [wip] Better handle pending deprecations Feb 16, 2023
@coveralls
Copy link

Pull Request Test Coverage Report for Build 4195565630

  • 9 of 11 (81.82%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.002%) to 85.271%

Changes Missing Coverage Covered Lines Changed/Added Lines %
qiskit/utils/deprecation.py 9 11 81.82%
Totals Coverage Status
Change from base Build 4194805799: -0.002%
Covered Lines: 67230
Relevant Lines: 78843

💛 - Coveralls

@woodsp-ibm
Copy link
Member

As you noted we use the deprecation util in the application repos as well. I will also point out that the set of changes done by this PR, around the algorithms, is also changed by #9532 which is intended for the next release.

@Eric-Arellano
Copy link
Collaborator Author

Thanks for that context, @woodsp-ibm. @manoelmarques and I DMed about their two open PRs - I'm going to put up soon a prework PR that builds off of their deprecation.py changes, then help adapt the PRs to use that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changelog: None Do not include in changelog refactor Proposal to refactor code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants