From df440b7ff10293ec74df5a8b95c2b4e19af70b6b Mon Sep 17 00:00:00 2001 From: iuliazidaru Date: Fri, 24 Dec 2021 11:49:03 +0200 Subject: [PATCH 1/4] dag_drawer should check the existence of filename extension --- qiskit/exceptions.py | 4 ++ qiskit/visualization/dag_visualization.py | 67 +++++++++++++++++++- test/python/visualization/test_dag_drawer.py | 27 ++++++++ 3 files changed, 97 insertions(+), 1 deletion(-) diff --git a/qiskit/exceptions.py b/qiskit/exceptions.py index d3a721804fa0..7ae4d1fccd92 100644 --- a/qiskit/exceptions.py +++ b/qiskit/exceptions.py @@ -73,3 +73,7 @@ def __init__( def __str__(self) -> str: """Return the message.""" return repr(self.message) + + +class InvalidFileError(QiskitError): + """Raised when the file provided is not valid for the specific task.""" diff --git a/qiskit/visualization/dag_visualization.py b/qiskit/visualization/dag_visualization.py index 8d443bcb025f..0ed0cdabb526 100644 --- a/qiskit/visualization/dag_visualization.py +++ b/qiskit/visualization/dag_visualization.py @@ -21,7 +21,7 @@ import tempfile from qiskit.dagcircuit.dagnode import DAGOpNode, DAGInNode, DAGOutNode -from qiskit.exceptions import MissingOptionalLibraryError +from qiskit.exceptions import MissingOptionalLibraryError, InvalidFileError from .exceptions import VisualizationError try: @@ -31,6 +31,63 @@ except ImportError: HAS_PIL = False +FILENAME_EXTENSIONS = [ + "bmp", + "canon", + "cgimage", + "cmap", + "cmapx", + "cmapx_np", + "dot", + "dot_json", + "eps exr", + "fig", + "gd", + "gd2", + "gif", + "gv", + "icns", + "ico", + "imap", + "imap_np", + "ismap", + "jp2", + "jpe", + "jpeg", + "jpg", + "json", + "json0", + "mp", + "pct", + "pdf", + "pic", + "pict", + "plain", + "plain-ext", + "png", + "pov", + "ps", + "ps2", + "psd", + "sgi", + "svg", + "svgz", + "tga", + "tif", + "tiff", + "tk", + "vdx", + "vml", + "vmlz", + "vrml", + "wbmp", + "webp", + "xdot", + "xdot1.2", + "xdot1.4", + "xdot_json", +] + def dag_drawer(dag, scale=0.7, filename=None, style="color"): """Plot the directed acyclic graph (dag) to represent operation dependencies @@ -59,6 +116,7 @@ def dag_drawer(dag, scale=0.7, filename=None, style="color"): Raises: VisualizationError: when style is not recognized. MissingOptionalLibraryError: when pydot or pillow are not installed. + InvalidFileError: when filename provided is not valid Example: .. jupyter-execute:: @@ -166,7 +224,14 @@ def edge_attr_func(edge): dot = pydot.graph_from_dot_data(dot_str)[0] if filename: + if "." not in filename: + raise InvalidFileError("Parameter 'filename' must be in format 'name.extension'") extension = filename.split(".")[-1] + if extension not in FILENAME_EXTENSIONS: + raise InvalidFileError( + "Filename extension must be one of: " + + " ".join([str(elem) for elem in FILENAME_EXTENSIONS]) + ) dot.write(filename, format=extension) return None elif ("ipykernel" in sys.modules) and ("spyder" not in sys.modules): diff --git a/test/python/visualization/test_dag_drawer.py b/test/python/visualization/test_dag_drawer.py index 2e2920c078ef..4a77a7500cbc 100644 --- a/test/python/visualization/test_dag_drawer.py +++ b/test/python/visualization/test_dag_drawer.py @@ -17,6 +17,7 @@ from qiskit import QuantumRegister, QuantumCircuit from qiskit.test import QiskitTestCase from qiskit.tools.visualization import dag_drawer +from qiskit.exceptions import InvalidFileError from qiskit.visualization.exceptions import VisualizationError from qiskit.converters import circuit_to_dag @@ -36,6 +37,32 @@ def test_dag_drawer_invalid_style(self): """Test dag draw with invalid style.""" self.assertRaises(VisualizationError, dag_drawer, self.dag, style="multicolor") + def test_dag_drawer_checks_filename_correct_format(self): + """filename must contain name and extension""" + try: + dag_drawer(self.dag, filename="aaabc") + self.fail("Expected error not raised!") + except InvalidFileError as exception_instance: + self.assertEqual( + exception_instance.message, + "Parameter 'filename' must be in format 'name.extension'", + ) + + def test_dag_drawer_checks_filename_extension(self): + """filename must have a valid extension""" + try: + dag_drawer(self.dag, filename="aa.abc") + self.fail("Expected error not raised!") + except InvalidFileError as exception_instance: + self.assertEqual( + exception_instance.message, + "Filename extension must be one of: bmp canon cgimage cmap cmapx cmapx_np " + "dot dot_json eps exr fig gd gd2 gif gv icns ico imap imap_np ismap jp2 " + "jpe jpeg jpg json json0 mp pct pdf pic pict plain plain-ext png pov " + "ps ps2 psd sgi svg svgz tga tif tiff tk vdx vml vmlz vrml wbmp webp " + "xdot xdot1.2 xdot1.4 xdot_json", + ) + if __name__ == "__main__": unittest.main(verbosity=2) From 27bd2294d6108074c52c6e31f026e9ab3039c4b5 Mon Sep 17 00:00:00 2001 From: iuliazidaru Date: Wed, 12 Jan 2022 15:35:19 +0200 Subject: [PATCH 2/4] fix review comments --- qiskit/visualization/dag_visualization.py | 10 ++++----- test/python/visualization/test_dag_drawer.py | 22 ++++---------------- 2 files changed, 9 insertions(+), 23 deletions(-) diff --git a/qiskit/visualization/dag_visualization.py b/qiskit/visualization/dag_visualization.py index 0ed0cdabb526..85ab227511bc 100644 --- a/qiskit/visualization/dag_visualization.py +++ b/qiskit/visualization/dag_visualization.py @@ -31,7 +31,7 @@ except ImportError: HAS_PIL = False -FILENAME_EXTENSIONS = [ +FILENAME_EXTENSIONS = { "bmp", "canon", "cgimage", @@ -40,7 +40,8 @@ "cmapx_np", "dot", "dot_json", - "eps exr", + "eps", + "exr", "fig", "gd", "gd2", @@ -86,7 +87,7 @@ "xdot1.2", "xdot1.4", "xdot_json", -] +} def dag_drawer(dag, scale=0.7, filename=None, style="color"): @@ -229,8 +230,7 @@ def edge_attr_func(edge): extension = filename.split(".")[-1] if extension not in FILENAME_EXTENSIONS: raise InvalidFileError( - "Filename extension must be one of: " - + " ".join([str(elem) for elem in FILENAME_EXTENSIONS]) + "Filename extension must be one of: " + " ".join(FILENAME_EXTENSIONS) ) dot.write(filename, format=extension) return None diff --git a/test/python/visualization/test_dag_drawer.py b/test/python/visualization/test_dag_drawer.py index 4a77a7500cbc..a34f96411069 100644 --- a/test/python/visualization/test_dag_drawer.py +++ b/test/python/visualization/test_dag_drawer.py @@ -39,29 +39,15 @@ def test_dag_drawer_invalid_style(self): def test_dag_drawer_checks_filename_correct_format(self): """filename must contain name and extension""" - try: + with self.assertRaisesRegex( + InvalidFileError, "Parameter 'filename' must be in format 'name.extension'" + ): dag_drawer(self.dag, filename="aaabc") - self.fail("Expected error not raised!") - except InvalidFileError as exception_instance: - self.assertEqual( - exception_instance.message, - "Parameter 'filename' must be in format 'name.extension'", - ) def test_dag_drawer_checks_filename_extension(self): """filename must have a valid extension""" - try: + with self.assertRaisesRegex(InvalidFileError, "Filename extension must be one of: .*"): dag_drawer(self.dag, filename="aa.abc") - self.fail("Expected error not raised!") - except InvalidFileError as exception_instance: - self.assertEqual( - exception_instance.message, - "Filename extension must be one of: bmp canon cgimage cmap cmapx cmapx_np " - "dot dot_json eps exr fig gd gd2 gif gv icns ico imap imap_np ismap jp2 " - "jpe jpeg jpg json json0 mp pct pdf pic pict plain plain-ext png pov " - "ps ps2 psd sgi svg svgz tga tif tiff tk vdx vml vmlz vrml wbmp webp " - "xdot xdot1.2 xdot1.4 xdot_json", - ) if __name__ == "__main__": From 74aded3bc4881e172a27f7ec9ba6f92412a5917c Mon Sep 17 00:00:00 2001 From: iuliazidaru Date: Fri, 14 Jan 2022 13:06:20 +0200 Subject: [PATCH 3/4] add release note --- ...wer-should-check-filename-existence-4a83418a893717f6.yaml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 releasenotes/notes/dag-drawer-should-check-filename-existence-4a83418a893717f6.yaml diff --git a/releasenotes/notes/dag-drawer-should-check-filename-existence-4a83418a893717f6.yaml b/releasenotes/notes/dag-drawer-should-check-filename-existence-4a83418a893717f6.yaml new file mode 100644 index 000000000000..80b28008654b --- /dev/null +++ b/releasenotes/notes/dag-drawer-should-check-filename-existence-4a83418a893717f6.yaml @@ -0,0 +1,5 @@ +--- +fixes: + - | + Function ``dag_drawer`` from ``qiskit.visualization`` now throws a specific exception when the filename + provided is invalid. From c12176793361d1f4ed912d7a61f8f9db9dc23d27 Mon Sep 17 00:00:00 2001 From: Jake Lishman Date: Fri, 14 Jan 2022 11:47:11 +0000 Subject: [PATCH 4/4] Reword release note --- ...wer-should-check-filename-existence-4a83418a893717f6.yaml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/releasenotes/notes/dag-drawer-should-check-filename-existence-4a83418a893717f6.yaml b/releasenotes/notes/dag-drawer-should-check-filename-existence-4a83418a893717f6.yaml index 80b28008654b..6f98468a32a0 100644 --- a/releasenotes/notes/dag-drawer-should-check-filename-existence-4a83418a893717f6.yaml +++ b/releasenotes/notes/dag-drawer-should-check-filename-existence-4a83418a893717f6.yaml @@ -1,5 +1,6 @@ --- fixes: - | - Function ``dag_drawer`` from ``qiskit.visualization`` now throws a specific exception when the filename - provided is invalid. + :meth:`.DAGCircuit.draw` and the associated function :func:`.dag_drawer` + will now show a more useful error message when the provided filename is not + valid.