Apply new deprecation decorators to visualization folder #9867
Conversation
|
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:
|
| DeprecationWarning, | ||
| 2, | ||
| ) | ||
| del reverse_bits |
There was a problem hiding this comment.
FYI the del is to make Pylint happy.
There was a problem hiding this comment.
I think we the previous decorator, it was possible to remove the kwarg that was deprecated... a non-blocker for this one.
| circ = QuantumCircuit(qubits, clbits) | ||
| for reg in qregs: | ||
| for reg in qregs or []: |
There was a problem hiding this comment.
This was a bug fix. If qregs is None, then you can't iterate over it.
There was a problem hiding this comment.
that's true, but passing neither qregs nor circuit is an invalid calling convention, so it's fine for it just to explode tbh. (It doesn't come up because this is actually just an internal-detail class.)
1ucian0
added
the
type: qa
| @@ -561,12 +561,8 @@ def _generate_latex_source( | |||
| style=style, | |||
| reverse_bits=reverse_bits, | |||
| plot_barriers=plot_barriers, | |||
| layout=None, | |||
There was a problem hiding this comment.
These lines in this file caused deprecation warnings. There's no need to set them because None is the default.
| "arguments for rendering the drawing." | ||
| ), | ||
| ) | ||
| def __init__( # pylint: disable=bad-docstring-quotes |
There was a problem hiding this comment.
I couldn't figure out why Pylint is mad. Sphinx is happy with the docstring, and I confirmed that QCircuitImage.__init__.__doc__ looks good.
It seems low priority to figure this out: what we really care about is Sphinx being happy.
There was a problem hiding this comment.
I'm like 80% sure it's a bug in pylint, but I don't have the issue to hand. If you search for bad-docstring-quotes in Terra I think there's a couple dozen instances.
Pull Request Test Coverage Report for Build 4863626625
💛 - Coveralls |
|
Bump |
| "arguments for rendering the drawing." | ||
| ), | ||
| ) | ||
| def __init__( # pylint: disable=bad-docstring-quotes |
There was a problem hiding this comment.
I'm like 80% sure it's a bug in pylint, but I don't have the issue to hand. If you search for bad-docstring-quotes in Terra I think there's a couple dozen instances.
| circ = QuantumCircuit(qubits, clbits) | ||
| for reg in qregs: | ||
| for reg in qregs or []: |
There was a problem hiding this comment.
that's true, but passing neither qregs nor circuit is an invalid calling convention, so it's fine for it just to explode tbh. (It doesn't come up because this is actually just an internal-detail class.)
| def _is_deprecated_data_format(data) -> bool: | ||
| if not isinstance(data, list): | ||
| data = [data] | ||
| for dat in data: | ||
| if isinstance(dat, (QuasiDistribution, ProbDistribution)) or isinstance( | ||
| next(iter(dat.values())), float | ||
| ): | ||
| return True | ||
| return False | ||
|
|
||
|
|
||
| @deprecate_arg( | ||
| "data", | ||
| deprecation_description=( | ||
| "Using plot_histogram() ``data`` argument with QuasiDistribution, ProbDistribution, or a " | ||
| "distribution dictionary" | ||
| ), | ||
| since="0.22.0", | ||
| additional_msg="Instead, use ``plot_distribution()``.", | ||
| predicate=_is_deprecated_data_format, | ||
| pending=True, | ||
| ) |
There was a problem hiding this comment.
This is starting to add non-trivial overhead into any call to the function, since we now loop through all the data now. It's not important for this visualisation function and doesn't need changing, but I think it's something we should be careful about in general.
There was a problem hiding this comment.
#9902 should avoid the duplication of being both in the decorator and the function body.
If you'd like, I can prioritize that for next week? Or remove this particular call?
There was a problem hiding this comment.
We can just leave this call in place for now - the inefficiency isn't going to be an issue for this particular function call.
mtreinish
added
the
stable backport potential
…ply-deprecations-visualization
* Apply new deprecation decorators to visualization folder * Remove stray backtick * Fix helper functions setting deprecated args to default of `None` * Ignore pylint issue * Don't use deprecator for `circuit is None` (cherry picked from commit e924ea9)
) * Apply new deprecation decorators to visualization folder * Remove stray backtick * Fix helper functions setting deprecated args to default of `None` * Ignore pylint issue * Don't use deprecator for `circuit is None` (cherry picked from commit e924ea9) Co-authored-by: Eric Arellano <14852634+Eric-Arellano@users.noreply.github.com> Co-authored-by: Luciano Bello <bel@zurich.ibm.com>
* Apply new deprecation decorators to visualization folder * Remove stray backtick * Fix helper functions setting deprecated args to default of `None` * Ignore pylint issue * Don't use deprecator for `circuit is None`
Summary
See #9676 for the motivation.
Details and comments