Legacy code deprecation in pass manager #11449
Conversation
nkanazawa1989
added
Changelog: Deprecation
|
One or more of the the following people are requested to review this:
|
There was a problem hiding this comment.
It looks like the correct things have been deprecated, but I think we need to revisit how it's been done:
- we should always use the
deprecate_funcdecorator to deprecate classes, so all the warnings are automatically raised correctly, and this also handles putting in.. deprecated::directives into the class documentation. - we oughtn't to be using
catch_warningsin library code. It adds extra runtime weight, but also, if the stacklevels are set correctly, it's unnecessary; the warnings will already be filtered out, and manually catching warnings prevents users from being able to control it themselves. We can filter the warnings that are blamed on ourselves in our own test suite, since we've already got the working version in Legacy code removal in pass manager #11448.
qiskit/transpiler/passmanager.py
Outdated
| with warnings.catch_warnings(): | ||
| warnings.filterwarnings("ignore", category=DeprecationWarning) | ||
| flatten_tasks = list(self._flatten_tasks(self._tasks)) | ||
| return RunningPassManager(flatten_tasks) |
There was a problem hiding this comment.
We shouldn't filter warnings within the library; it adds cost, but also, it's generally unnecessary - if we ensure that the warnings have their stack level set correctly, they'll blame Qiskit code and so not be shown to users by default.
There was a problem hiding this comment.
I removed almost all of these in 35452c5, by effectively pulling in the changes on #11448 into this branch. The particular one highlighted here has a gross hack to make internal library use work, without modifying downstream use at all, but downstream use of to_flow_controller should correctly see the warnings as being blamed on Qiskit code, so they should be fine to suppress those.
There was a problem hiding this comment.
Thanks Jake. I didn't came up with this solution. I've learned a lot :)
The intention here is to avoid overriding warning filters within our code, and just eagerly migrate to the preferred paths we already know to be identical. The `transpiler.PassManager.to_flow_controller` hack is a bit gross.
There was a problem hiding this comment.
I pushed non-trivial changes to this and it wouldn't hurt to have another reviewer, but since everyone's swamped, we're already on top of the 0.46 deadline, and especially since 0.46 still has the old CI workflows, I'm going to move this to merge and we can modify it afterwards if anything needed changing (assuming I didn't bork the tests or lint). Thanks for getting it sorted, Naoki.
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
Summary
This PR is deprecation version of #11448 for stable/0.46.
Details and comments