-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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 qiskit.tools module #11515
Remove qiskit.tools module #11515
Conversation
One or more of the the following people are requested to review this:
|
This commit removes all the functionality in `qiskit.tools` with the exception of `parallel_map` which has been migrated to `qiskit.utils`. This PR does not cover the removal of `qiskit.tools.jupyter` or `qiskit.tools.monitor` which are covered by Qiskit#11513.
9ab52d4
to
71748df
Compare
Pull Request Test Coverage Report for Build 7494204060
💛 - Coveralls |
…orever-from-this-point-forward
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems pleasantly small and an easy clean removal. The move of parallel_map
is a shade unfortunate, but we talked about that on #11514.
Publisher().publish("terra.parallel.start", len(values)) | ||
nfinished = [0] | ||
|
||
def _callback(_): | ||
nfinished[0] += 1 | ||
Publisher().publish("terra.parallel.done", nfinished[0]) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What was the point of this, historically?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was originally used to have a progress bar for things like parallel_map
. Overall there was a goal to have a decoupled event bus that different components could subscribe to for async processing. But it never was really used for anything more than progress bars.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, good to know, thanks. It was one of the major differences from the version that lived in QuTiP, but I'd never actually looked into it to find out what it was for.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(meant to click approve on that review)
Co-authored-by: Jake Lishman <[email protected]>
* Remove qiskit.tools module This commit removes all the functionality in `qiskit.tools` with the exception of `parallel_map` which has been migrated to `qiskit.utils`. This PR does not cover the removal of `qiskit.tools.jupyter` or `qiskit.tools.monitor` which are covered by Qiskit#11513. * Update qiskit/utils/__init__.py Co-authored-by: Jake Lishman <[email protected]> --------- Co-authored-by: Jake Lishman <[email protected]>
Summary
This commit removes all the functionality in
qiskit.tools
with the exception ofparallel_map
which has been migrated toqiskit.utils
. This PR does not cover the removal ofqiskit.tools.jupyter
orqiskit.tools.monitor
which are covered by #11513.Details and comments
This should probably merge after #11513, there shouldn't be a merge conflict (if I did it correctly), but the removal is incomplete without #11513 and I expect there will be some test failures related to that.