Skip to content

Commit

Permalink
Add warning for bad justify input, in circuit_drawer (#12458)
Browse files Browse the repository at this point in the history
* add warning for bad justify input in circuit_drawer

* change justify after raising error

* typo indentation + improving warning string

* Undo max_lenght limit by autoformater (120 > 105)

* Make lines fullfill 105 max lenght requirement

* Solve regex matching parenthesis problem and don't trigger the wanring for default value

* Change justify default value to "left", & add deprecation wrapper

* Change and extend warnings tests

* Solve various layers of same argument DeprecationWarning

* Added a clarification comment for the solution, about multiple deprecationwarnings

* Ignore cyclic import error, as above

* Apply suggestions from code review

Co-authored-by: Eric Arellano <[email protected]>

* Apply suggestions from code review

* Improve DeprecationWarning readability, and fix warning checks tests

* Remove `_is_valid_justify_arg` from `@deprecate_arg`, for solving circular import

* in `deprecate_arg` change `since` to "1.2.0"

* black formater suggestion

* negate conditions for `predicate` in `@deprecate_arg`

* remove `pending=True`, since then warning is not raised

* change qiskit version on tests

* remove assert Regex for DeprecationWarning

* Add release note, and remove two undesired changes in imports

* changing release note naming from "_" to "-"

* Add extra line in the end, for lint

* Redid release file from start, with shorter name, and correct spacings

* Remove final spaces in all lines...

* Try without deprecations_visualization section..

* Solve, bad Sphinx spacing, go back to deprecations_visualization

* Go back to `None` as default value

* Simplifying deprecation logic

* Remove unused imports and changed missing default value

* Improve docstring for public methods

* Improve error readbility and testing of it with regex

---------

Co-authored-by: Eric Arellano <[email protected]>
  • Loading branch information
GuillermoAbadLopez and Eric-Arellano authored Jul 1, 2024
1 parent 67fd35a commit ed87f2f
Show file tree
Hide file tree
Showing 5 changed files with 84 additions and 28 deletions.
10 changes: 5 additions & 5 deletions qiskit/circuit/quantumcircuit.py
Original file line number Diff line number Diff line change
Expand Up @@ -3257,11 +3257,11 @@ def draw(
alternative value set. For example, ``circuit_reverse_bits = True``.
plot_barriers: Enable/disable drawing barriers in the output
circuit. Defaults to ``True``.
justify: Options are ``left``, ``right`` or ``none``. If
anything else is supplied, it defaults to left justified. It refers
to where gates should be placed in the output circuit if there is
an option. ``none`` results in each gate being placed in its own
column.
justify: Options are ``"left"``, ``"right"`` or ``"none"`` (str).
If anything else is supplied, left justified will be used instead.
It refers to where gates should be placed in the output circuit if
there is an option. ``none`` results in each gate being placed in
its own column. Defaults to ``left``.
vertical_compression: ``high``, ``medium`` or ``low``. It
merges the lines generated by the `text` output so the drawing
will take less vertical room. Default is ``medium``. Only used by
Expand Down
42 changes: 33 additions & 9 deletions qiskit/visualization/circuit/_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,21 +14,25 @@

import re
from collections import OrderedDict
from warnings import warn

import numpy as np

from qiskit.circuit import (
ClassicalRegister,
Clbit,
ControlFlowOp,
ControlledGate,
Delay,
Gate,
Instruction,
Measure,
QuantumCircuit,
Qubit,
)
from qiskit.circuit.annotated_operation import AnnotatedOperation, InverseModifier, PowerModifier
from qiskit.circuit.controlflow import condition_resources
from qiskit.circuit.library import PauliEvolutionGate
from qiskit.circuit import ClassicalRegister, QuantumCircuit, Qubit, ControlFlowOp
from qiskit.circuit.annotated_operation import AnnotatedOperation, InverseModifier, PowerModifier
from qiskit.circuit.tools import pi_check
from qiskit.converters import circuit_to_dag
from qiskit.utils import optionals as _optionals
Expand Down Expand Up @@ -370,6 +374,29 @@ def generate_latex_label(label):
return final_str.replace(" ", "\\,") # Put in proper spaces


def _get_valid_justify_arg(justify):
"""Returns a valid `justify` argument, warning if necessary."""
if isinstance(justify, str):
justify = justify.lower()

if justify is None:
justify = "left"

if justify not in ("left", "right", "none"):
# This code should be changed to an error raise, once the deprecation is complete.
warn(
f"Setting QuantumCircuit.draw()’s or circuit_drawer()'s justify argument: {justify}, to a "
"value other than 'left', 'right', 'none' or None (='left'). Default 'left' will be used. "
"Support for invalid justify arguments is deprecated as of qiskit 1.2.0. Starting no "
"earlier than 3 months after the release date, invalid arguments will error.",
DeprecationWarning,
2,
)
justify = "left"

return justify


def _get_layered_instructions(
circuit, reverse_bits=False, justify=None, idle_wires=True, wire_order=None, wire_map=None
):
Expand All @@ -384,21 +411,18 @@ def _get_layered_instructions(
reverse_bits (bool): If true the order of the bits in the registers is
reversed.
justify (str) : `left`, `right` or `none`. Defaults to `left`. Says how
the circuit should be justified.
the circuit should be justified. If an invalid value is provided,
default `left` will be used.
idle_wires (bool): Include idle wires. Default is True.
wire_order (list): A list of ints that modifies the order of the bits
wire_order (list): A list of ints that modifies the order of the bits.
Returns:
Tuple(list,list,list): To be consumed by the visualizer directly.
Raises:
VisualizationError: if both reverse_bits and wire_order are entered.
"""
if justify:
justify = justify.lower()

# default to left
justify = justify if justify in ("right", "none") else "left"
justify = _get_valid_justify_arg(justify)

if wire_map is not None:
qubits = [bit for bit in wire_map if isinstance(bit, Qubit)]
Expand Down
23 changes: 12 additions & 11 deletions qiskit/visualization/circuit/circuit_visualization.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,21 +28,22 @@

import logging
import os
import shutil
import subprocess
import tempfile
import shutil
import typing
from warnings import warn

from qiskit import user_config
from qiskit.utils import optionals as _optionals
from qiskit.circuit import ControlFlowOp, Measure
from qiskit.utils import optionals as _optionals

from ..exceptions import VisualizationError
from ..utils import _trim as trim_image
from . import _utils
from . import latex as _latex
from . import text as _text
from . import matplotlib as _matplotlib
from . import _utils
from ..utils import _trim as trim_image
from ..exceptions import VisualizationError
from . import text as _text

if typing.TYPE_CHECKING:
from typing import Any
Expand Down Expand Up @@ -131,11 +132,11 @@ def circuit_drawer(
alternative value set. For example, ``circuit_reverse_bits = True``.
plot_barriers: Enable/disable drawing barriers in the output
circuit. Defaults to ``True``.
justify: Options are ``left``, ``right`` or ``none``. If
anything else is supplied, it defaults to left justified. It refers
to where gates should be placed in the output circuit if there is
an option. ``none`` results in each gate being placed in its own
column.
justify: Options are ``"left"``, ``"right"`` or ``"none"`` (str).
If anything else is supplied, left justified will be used instead.
It refers to where gates should be placed in the output circuit if
there is an option. ``none`` results in each gate being placed in
its own column. Defaults to ``left``.
vertical_compression: ``high``, ``medium`` or ``low``. It
merges the lines generated by the `text` output so the drawing
will take less vertical room. Default is ``medium``. Only used by
Expand Down
13 changes: 13 additions & 0 deletions releasenotes/notes/circuit-draw-warn-justify-03434d30cccda452.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
---
deprecations_visualization:
- |
The ``justify`` argument of :func:`circuit_drawer` or :meth:`QuantumCircuit.draw`, will
no longer support invalid values (previously changing them to the default), and in a future
release they will error. Valid justify values are ``"left"``, ``"right"`` or ``"none"``.
fixes:
- |
Fixed an issue where :func:`circuit_drawer` or the :meth:`QuantumCircuit.draw` method would
not raise a warning when an invalid value was passed to the ``justify`` argument, before
changing it to the default. Now, it will raise a warning if an invalid value is passed.
Valid justify values are ``"left"``, ``"right"`` or ``"none"``. Refer to
`#12089 <https://github.com/Qiskit/qiskit/issues/12089>` for more details.
24 changes: 21 additions & 3 deletions test/python/visualization/test_circuit_drawer.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,16 +14,16 @@

import os
import pathlib
import re
import shutil
import tempfile
import unittest
import warnings
from unittest.mock import patch

from qiskit import QuantumCircuit, QuantumRegister, ClassicalRegister
from qiskit import ClassicalRegister, QuantumCircuit, QuantumRegister, visualization
from qiskit.utils import optionals
from qiskit import visualization
from qiskit.visualization.circuit import text, styles
from qiskit.visualization.circuit import styles, text
from qiskit.visualization.exceptions import VisualizationError
from test import QiskitTestCase # pylint: disable=wrong-import-order

Expand Down Expand Up @@ -241,6 +241,24 @@ def test_reverse_bits(self):
result = visualization.circuit_drawer(circuit, output="text", reverse_bits=True)
self.assertEqual(result.__str__(), expected)

def test_warning_for_bad_justify_argument(self):
"""Test that the correct DeprecationWarning is raised when the justify parameter is badly input,
for both of the public interfaces."""
circuit = QuantumCircuit()
bad_arg = "bad"
error_message = re.escape(
f"Setting QuantumCircuit.draw()’s or circuit_drawer()'s justify argument: {bad_arg}, to a "
"value other than 'left', 'right', 'none' or None (='left'). Default 'left' will be used. "
"Support for invalid justify arguments is deprecated as of qiskit 1.2.0. Starting no "
"earlier than 3 months after the release date, invalid arguments will error.",
)

with self.assertWarnsRegex(DeprecationWarning, error_message):
visualization.circuit_drawer(circuit, justify=bad_arg)

with self.assertWarnsRegex(DeprecationWarning, error_message):
circuit.draw(justify=bad_arg)

@unittest.skipUnless(optionals.HAS_PYLATEX, "needs pylatexenc for LaTeX conversion")
def test_no_explict_cregbundle(self):
"""Test no explicit cregbundle should not raise warnings about being disabled
Expand Down

0 comments on commit ed87f2f

Please sign in to comment.