Change algorithms from pending deprecation to deprecated #9883

Eric-Arellano · 2023-03-30T16:03:26Z

Summary

Supersedes #9532.

Details and comments

Co-authored-by: Manoel Marques <manoelmrqs@gmail.com>

qiskit-bot · 2023-03-30T16:03:30Z

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
@manoelmarques
@woodsp-ibm

Eric-Arellano · 2023-03-30T16:03:56Z

WIP because I want to see what test changes from #9532 are necessary.

Feel free to start reviewing the prod changes though!

woodsp-ibm · 2023-03-30T18:19:46Z

qiskit/algorithms/amplitude_amplifiers/grover.py

@@ -181,7 +183,7 @@ def __init__(
        self._quantum_instance = None
        if quantum_instance is not None:
            with warnings.catch_warnings():
-                warnings.simplefilter("ignore", category=PendingDeprecationWarning)
+                warnings.simplefilter("ignore")


I see this was there before but I don't think we should do this. I do not believe the error message from the setter will be seen under normal usage, only something that is directly called like when calling the init here.

I think it will be rendered because we indeed are calling the setter. I don't see much downside to these 2 lines, but there's a big upside in being confident they won't see a "double warning". The @deprecate_arg here is all we want them to see.

But I'll add back the category argument.

I am pretty sure that nowadays they will not - it used to be Terra has some config set that all deprecations were shown. And if you configure things to see deprecations elsewhere we will have swallowed this. With the amount deprecation around opflow I cannot believe this is the only time internally we are calling/using deprecated code. It should be easy enough to check whether you see this or not. Create a Grover with a plain Backend.

qiskit/algorithms/optimizers/qnspsa.py

…precations-algorithms-pending

coveralls · 2023-03-31T19:32:32Z

Pull Request Test Coverage Report for Build 4699944194

30 of 30 (100.0%) changed or added relevant lines in 16 files are covered.
9 unchanged lines in 3 files lost coverage.
Overall coverage increased (+0.05%) to 85.798%

Files with Coverage Reduction	New Missed Lines	%
qiskit/extensions/quantum_initializer/squ.py	2	80.0%
crates/qasm2/src/lex.rs	3	91.14%
crates/accelerate/src/sabre_swap/layer.rs	4	97.32%

Totals
Change from base Build 4699523725:	0.05%
Covered Lines:	70455
Relevant Lines:	82117

💛 - Coveralls

woodsp-ibm

@Eric-Arellano Now that the algorithm migration guide is done I think we should be able to finalize/merge this. It needs a conflict sorting and I left a couple of comments as well. Otherwise it all seems good to go right? Thanks again for doing this for us!

qiskit/algorithms/amplitude_amplifiers/grover.py

releasenotes/notes/deprecate-algorithms-c6e1e28b6091c507.yaml

…precations-algorithms-pending

woodsp-ibm

LGTM thx again!

woodsp-ibm · 2023-04-14T19:51:37Z

I will make a minor comment about the text though. Here is what comes out in the VQE class when you look at its docs. We talked about having refs in here instead so its not that. But when I read the first part The class xxxxx.. it almost sounds like some other class especially at the moment with that long ref there - I wonder whether This class xxxxx... would work out better for the docs here so it more seems to refer to what you are currently reading and would be ok in the error message too.

Eric-Arellano · 2023-04-14T20:06:02Z

I think I like this class when it's in the docs. But "This class" maybe reads weird during a runtime warning?

FYI, if I recall correctly, I switched to the full module name because I encountered some deprecated function names that were the same function name but in different modules. I wanted to remove all ambiguity.

woodsp-ibm · 2023-04-14T20:15:38Z

But "This class" maybe reads weird during a runtime warning?

Yes, that's kinda why I said work out better for the docs. Its a tradeoff given the duality of usage. Since the runtime message is coming from the class itself I though it would be kinda ok that it has this class - it does name itself too. It just seemed weirder in the docs to me where you kinda have to realize that "The class..." is what you are reading and not some other its referring to. The long ref in there at present did not help in that regard for me. Anyway its pretty minor. Its nice to have the notice coming out in the docs and something that is easily tweaked in the decorator right. Maybe its just me - can see what others think going forwards too.

Eric-Arellano · 2023-04-14T20:59:23Z

Its nice to have the notice coming out in the docs and something that is easily tweaked in the decorator right.

Yes, very easy to change at any time.

* Change algorithms from pending deprecation to deprecated Co-authored-by: Manoel Marques <manoelmrqs@gmail.com> * Review feedback & fix fmt * Try to fix the tests * Another round of test fixes * Last round of test fixes? * Actual last test fix! * Use HTTPS * Refer to migration guide in deprecation changelog --------- Co-authored-by: Manoel Marques <manoelmrqs@gmail.com>

…it#9883) * Change algorithms from pending deprecation to deprecated Co-authored-by: Manoel Marques <manoelmrqs@gmail.com> * Review feedback & fix fmt * Try to fix the tests * Another round of test fixes * Last round of test fixes? * Actual last test fix! * Use HTTPS * Refer to migration guide in deprecation changelog --------- Co-authored-by: Manoel Marques <manoelmrqs@gmail.com>

Change algorithms from pending deprecation to deprecated

f1115b7

Co-authored-by: Manoel Marques <manoelmrqs@gmail.com>

woodsp-ibm reviewed Mar 30, 2023

View reviewed changes

qiskit/algorithms/optimizers/qnspsa.py Outdated Show resolved Hide resolved

Eric-Arellano added 7 commits March 31, 2023 08:43

Merge branch 'main' of https://github.com/Qiskit/qiskit-terra into de…

61f41ed

…precations-algorithms-pending

Review feedback & fix fmt

ee64090

Try to fix the tests

66eb03c

Another round of test fixes

b43f505

Merge branch 'main' of https://github.com/Qiskit/qiskit-terra into de…

29b2d35

…precations-algorithms-pending

Last round of test fixes?

01d7aad

Actual last test fix!

9484f4f

Eric-Arellano changed the title ~~[wip] Change algorithms from pending deprecation to deprecated~~ Change algorithms from pending deprecation to deprecated Mar 31, 2023

Eric-Arellano marked this pull request as ready for review March 31, 2023 19:02

Eric-Arellano requested review from a team and ElePT as code owners March 31, 2023 19:02

woodsp-ibm mentioned this pull request Apr 5, 2023

Bad import of the VQE class when importing from algorithms #9894

Closed

Eric-Arellano added this to the 0.24.0 milestone Apr 10, 2023

mtreinish added the priority: high label Apr 11, 2023

woodsp-ibm added the mod: algorithms Related to the Algorithms module label Apr 13, 2023

woodsp-ibm reviewed Apr 13, 2023

View reviewed changes

qiskit/algorithms/amplitude_amplifiers/grover.py Outdated Show resolved Hide resolved

releasenotes/notes/deprecate-algorithms-c6e1e28b6091c507.yaml Outdated Show resolved Hide resolved

1ucian0 assigned woodsp-ibm Apr 13, 2023

Eric-Arellano added 3 commits April 14, 2023 06:40

Merge branch 'main' of https://github.com/Qiskit/qiskit-terra into de…

808bf29

…precations-algorithms-pending

Use HTTPS

90eca87

Refer to migration guide in deprecation changelog

4b0e3ae

woodsp-ibm approved these changes Apr 14, 2023

View reviewed changes

Eric-Arellano added this pull request to the merge queue Apr 14, 2023

Merged via the queue into Qiskit:main with commit 3ebef17 Apr 14, 2023

woodsp-ibm mentioned this pull request Apr 14, 2023

Change algorithms from pending deprecate to deprecate #9532

Closed

Eric-Arellano deleted the deprecations-algorithms-pending branch April 14, 2023 23:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Change algorithms from pending deprecation to deprecated #9883

Change algorithms from pending deprecation to deprecated #9883

Eric-Arellano commented Mar 30, 2023 •

edited by woodsp-ibm

Loading

qiskit-bot commented Mar 30, 2023

Eric-Arellano commented Mar 30, 2023

woodsp-ibm Mar 30, 2023

Eric-Arellano Mar 31, 2023

woodsp-ibm Mar 31, 2023

coveralls commented Mar 31, 2023 •

edited

Loading

woodsp-ibm left a comment •

edited

Loading

woodsp-ibm left a comment

woodsp-ibm commented Apr 14, 2023

Eric-Arellano commented Apr 14, 2023

woodsp-ibm commented Apr 14, 2023

Eric-Arellano commented Apr 14, 2023

Change algorithms from pending deprecation to deprecated #9883

Change algorithms from pending deprecation to deprecated #9883

Conversation

Eric-Arellano commented Mar 30, 2023 • edited by woodsp-ibm Loading

Summary

Details and comments

qiskit-bot commented Mar 30, 2023

Eric-Arellano commented Mar 30, 2023

woodsp-ibm Mar 30, 2023

Choose a reason for hiding this comment

Eric-Arellano Mar 31, 2023

Choose a reason for hiding this comment

woodsp-ibm Mar 31, 2023

Choose a reason for hiding this comment

coveralls commented Mar 31, 2023 • edited Loading

Pull Request Test Coverage Report for Build 4699944194

💛 - Coveralls

woodsp-ibm left a comment • edited Loading

Choose a reason for hiding this comment

woodsp-ibm left a comment

Choose a reason for hiding this comment

woodsp-ibm commented Apr 14, 2023

Eric-Arellano commented Apr 14, 2023

woodsp-ibm commented Apr 14, 2023

Eric-Arellano commented Apr 14, 2023

Eric-Arellano commented Mar 30, 2023 •

edited by woodsp-ibm

Loading

coveralls commented Mar 31, 2023 •

edited

Loading

woodsp-ibm left a comment •

edited

Loading