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

Deprecate warning box in docs #8600

Closed
wants to merge 54 commits into from
Closed

Conversation

1ucian0
Copy link
Member

@1ucian0 1ucian0 commented Aug 23, 2022

Summary

Following #1592, this PR extend deprecate_arguments and deprecate_function so they add a deprecation box note in the docstring.

@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:

  • @Qiskit/terra-core

@coveralls
Copy link

coveralls commented Aug 23, 2022

Pull Request Test Coverage Report for Build 4005589012

  • 75 of 76 (98.68%) changed or added relevant lines in 7 files are covered.
  • 65 unchanged lines in 2 files lost coverage.
  • Overall coverage increased (+0.04%) to 84.937%

Changes Missing Coverage Covered Lines Changed/Added Lines %
qiskit/utils/deprecation.py 63 64 98.44%
Files with Coverage Reduction New Missed Lines %
src/sabre_swap/layer.rs 2 98.95%
qiskit/visualization/state_visualization.py 63 57.41%
Totals Coverage Status
Change from base Build 4001377713: 0.04%
Covered Lines: 66729
Relevant Lines: 78563

💛 - Coveralls

@kdk
Copy link
Member

kdk commented Aug 24, 2022

Can you post a screenshot of how this will show up in the docs?

@1ucian0
Copy link
Member Author

1ucian0 commented Aug 28, 2022

From @kdk:

Can you post a screenshot of how this will show up in the docs?

Because of Qiskit/qiskit_sphinx_theme#71, the current look is not the best.

@1ucian0 1ucian0 marked this pull request as ready for review August 28, 2022 09:37
@1ucian0
Copy link
Member Author

1ucian0 commented Sep 6, 2022

On hold, since qiskit_sphinx_theme needs to be released with Qiskit/qiskit_sphinx_theme#71 in order to support this. And current qiskit_sphinx_theme req needs to be bump.

@1ucian0 1ucian0 added on hold Can not fix yet documentation Something is not clear or an error documentation labels Sep 6, 2022
@1ucian0
Copy link
Member Author

1ucian0 commented Sep 6, 2022

qiskit_sphinx_theme 1.9 released. removing on hold.

@1ucian0 1ucian0 removed the on hold Can not fix yet label Sep 6, 2022
@1ucian0
Copy link
Member Author

1ucian0 commented Sep 6, 2022

Can you post a screenshot of how this will show up in the docs?

For deprecate_function:
Screenshot 2022-09-06 at 22 06 50

For deprecate_argument:
Screenshot 2022-09-07 at 15 24 44

@1ucian0 1ucian0 added this to the 0.23.0 milestone Nov 23, 2022
@1ucian0 1ucian0 requested a review from ElePT as a code owner January 11, 2023 16:53
@mtreinish mtreinish removed this from the 0.23.0 milestone Jan 24, 2023
@1ucian0 1ucian0 added this to the 0.24.0 milestone Jan 25, 2023
@1ucian0
Copy link
Member Author

1ucian0 commented Feb 6, 2023

While we all agree with the final result. This implementation penalises import time and requiere a parser (either "in-house" or a dependency). So new plan is:

@1ucian0
Copy link
Member Author

1ucian0 commented Feb 27, 2023

Closing this in favor of #9616

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Something is not clear or an error documentation on hold Can not fix yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants