Change style of DAG generated in dag_drawer() #13253

emilkovacev · 2024-10-01T21:25:45Z

Summary

Adds non-breaking interface to dag_drawer() to allow Qiskit users to customize the colors of DAGs.

Details and comments

I created stylesheets for dag_drawer(), that works similarly to how circuit_drawer does.

Here is an example:

from qiskit import QuantumRegister, ClassicalRegister, QuantumCircuit
from qiskit.converters import circuit_to_dag
from qiskit.visualization import dag_drawer
 

q = QuantumRegister(3, 'q')
c = ClassicalRegister(3, 'c')
circ = QuantumCircuit(q, c)
circ.h(q[0])
circ.cx(q[0], q[1])
circ.measure(q[0], c[0])
circ.rz(0.5, q[1]).c_if(c, 2)

dag = circuit_to_dag(circ)

style = {
    # "fontsize": 12,
    # "bgcolor": "white",
    # "dpi": 10,
    # "pad": 0,
    # "nodecolor": "green",
    "inputnodecolor": "pink",
    # "inputnodefontcolor": "white",
    "outputnodecolor": "lightblue",
    # "outputnodefontcolor": "white",
    "opnodecolor": "red",
    # "opnodefontcolor": "white",
    # "edgecolor": "black",
    # "qubitedgecolor": "red",
    # "clbitedgecolor": "black",
}

dag_drawer(dag, style=style)

The code above generates this DAG:

…r maximum customizability

qiskit-bot · 2024-10-01T21:25:50Z

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 following people are relevant to this code:

@enavarro51
@Qiskit/terra-core

coveralls · 2024-10-01T22:22:44Z

Pull Request Test Coverage Report for Build 12821657503

Details

18 of 167 (10.78%) changed or added relevant lines in 2 files are covered.
22 unchanged lines in 3 files lost coverage.
Overall coverage decreased (-0.09%) to 88.829%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
qiskit/visualization/dag/dagstyle.py	16	63	25.4%
qiskit/visualization/dag_visualization.py	2	104	1.92%

Files with Coverage Reduction	New Missed Lines	%
crates/qasm2/src/lex.rs	3	91.48%
qiskit/visualization/dag_visualization.py	7	9.64%
crates/qasm2/src/parse.rs	12	96.69%

Totals
Change from base Build 12813106471:	-0.09%
Covered Lines:	79457
Relevant Lines:	89449

💛 - Coveralls

mtreinish

Thanks for getting this started. I left an inline comment about the high level interface. I'm a bit concerned with how low level the current interface is and that it would require understanding how graphviz is used internally by dag_drawer

qiskit/visualization/dag_visualization.py

* Added common style templates for DAGs in qiskit/visualization/dag/styles * Loaded styles from stylesheets to color DAG * Fixed bug where 'plain' DAGs did not label nodes

raynelfss

This looks great, thank you for these additions. I wonder whether we should consider using FileSpecifier (such as File, Path, etc) as an alternative input for load_style(). I feel like it might benefit from not having to check whether the file exists or not. See the comments in my review.

raynelfss · 2024-11-04T17:21:53Z

qiskit/visualization/dag/dagstyle.py

+        else:
+            style_name = style
+    else:
+        raise exceptions.VisualizationError("Invalid style {style}")


I believe this might have been an f-string?

Suggested change

raise exceptions.VisualizationError("Invalid style {style}")

raise exceptions.VisualizationError(f"Invalid style {style}")

raynelfss · 2024-11-04T17:24:33Z

qiskit/visualization/dag/dagstyle.py

+        self.style = StyleDict(**default_style)
+
+
+def load_style(style: dict | str = "color") -> StyleDict:


Should we instead be able to provide a file or directory here too? Since whenever we provide a string it searches for one. Perhaps we should be able to specify a specific Path or file.

I agree, that seems a lot cleaner. This function definition was taken from the load_style() function in qiskit/visualization/circuit/qcstyle.py, should that be changed too or is that outside of the scope for this PR?

(I also noticed that the docstring here needs updating, I'll fix that too)

raynelfss · 2024-11-04T17:28:47Z

qiskit/visualization/dag/dagstyle.py

+    if style_name in ["color"]:
+        current_style = DefaultStyle().style


Maybe compere the strings directly here?

Suggested change

if style_name in ["color"]:

current_style = DefaultStyle().style

if style_name == "color":

current_style = DefaultStyle().style

raynelfss · 2024-11-04T17:31:10Z

qiskit/visualization/dag/dagstyle.py

+    else:
+        # Search for file in 'styles' dir, and then the current directory
+        style_name = style_name + ".json"
+        style_paths = []
+
+        default_path = Path(__file__).parent / "styles" / style_name
+        style_paths.append(default_path)
+
+        # check current directory
+        cwd_path = Path("") / style_name
+        style_paths.append(cwd_path)
+
+        for path in style_paths:
+            # expand ~ to the user directory and check if the file exists
+            exp_user = path.expanduser()
+            if os.path.isfile(exp_user):
+                try:
+                    with open(exp_user) as infile:
+                        json_style = json.load(infile)
+
+                    current_style = StyleDict(json_style)
+                    break
+                except json.JSONDecodeError as err:
+                    warn(
+                        f"Could not decode JSON in file '{path}': {str(err)}. "
+                        "Will use default style.",
+                        UserWarning,
+                        2,
+                    )
+                    break
+                except (OSError, FileNotFoundError):
+                    warn(
+                        f"Error loading JSON file '{path}'. Will use default style.",
+                        UserWarning,
+                        2,
+                    )
+                    break


This here can get a little convoluted. While providing a string name for a style that might be located at the styles directory is a good idea. We should be able to directly provide an instance of File or Path so that we don't need to perform this search actively and the user can just provide the Path themselves.

I agree, similar to before this code uses the same logic as the other instance of load_style() in qiskit/visualization/circuit/qcstyle.py, but with small changes to accommodate for the different use case.

raynelfss · 2024-11-04T19:16:16Z

qiskit/visualization/dag_visualization.py

-                if node.name == "barrier":
-                    n["style"] = "filled"
-                    n["fillcolor"] = "grey"
-                elif getattr(node.op, "_directive", False):
-                    n["style"] = "filled"
-                    n["fillcolor"] = "red"


Is there a reason why these two conditions were removed here?

emilkovacev added 6 commits October 1, 2024 00:41

Added 'custom' style that exposes graphviz customization functions fo…

e6e5caa

…r maximum customizability

Added type hints to function calls

3000fb8

Added docs

b92bfb2

Adds tests for dag_drawer

fd6a8a9

Fixed invalid exceptions caught by test

9ca881a

ran formatter and linter on code

68cd6f3

emilkovacev requested review from nonhermitian and a team as code owners October 1, 2024 21:25

qiskit-bot added the Community PR PRs from contributors that are not 'members' of the Qiskit repo label Oct 1, 2024

Merge branch 'main' into main

f0c8336

emilkovacev and others added 3 commits October 2, 2024 00:28

Formatted code and test to abide by pylint standards

a8a5c6a

Merge branch 'main' of github.com:emilkovacev/qiskit

4bae3af

Fix typo in docs for graph_attr kwarg

42680e1

mtreinish reviewed Oct 4, 2024

View reviewed changes

qiskit/visualization/dag_visualization.py Outdated Show resolved Hide resolved

emilkovacev and others added 11 commits October 8, 2024 21:24

placed functionality into stylesheets

a450cbc

Added error correction to dag visualization

a32493a

rewrote tests to work with new stylesheet method

7f29944

Added color templates for DAGs

59870e2

* Added common style templates for DAGs in qiskit/visualization/dag/styles * Loaded styles from stylesheets to color DAG * Fixed bug where 'plain' DAGs did not label nodes

Re-format docs for dag_visualization

c9ddc22

reformatted dag_visualization.py

37fdee4

Ran linting

7f118bb

Added artifacts for docs to recognize stylesheets

617d0ea

Fixed errors to match original invocation of dag_drawer()

ceab704

Fixed small import formatting issue

616c67e

Adds release notes for PR

379c4e1

emilkovacev requested a review from mtreinish October 13, 2024 03:06

raynelfss added this to the 1.3.0 milestone Oct 31, 2024

raynelfss removed the Community PR PRs from contributors that are not 'members' of the Qiskit repo label Oct 31, 2024

raynelfss self-assigned this Oct 31, 2024

raynelfss reviewed Nov 4, 2024

View reviewed changes

raynelfss modified the milestones: 1.3.0, 2.0.0 Nov 7, 2024

Merge branch 'main' of github.com:Qiskit/qiskit

1079dd8

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Change style of DAG generated in dag_drawer() #13253

Change style of DAG generated in dag_drawer() #13253

emilkovacev commented Oct 1, 2024 •

edited

Loading

qiskit-bot commented Oct 1, 2024

coveralls commented Oct 1, 2024 •

edited

Loading

mtreinish left a comment

raynelfss left a comment

raynelfss Nov 4, 2024

raynelfss Nov 4, 2024

emilkovacev Nov 9, 2024

raynelfss Nov 4, 2024

raynelfss Nov 4, 2024

emilkovacev Nov 9, 2024 •

edited

Loading

raynelfss Nov 4, 2024

	raise exceptions.VisualizationError("Invalid style {style}")
	raise exceptions.VisualizationError(f"Invalid style {style}")

		self.style = StyleDict(**default_style)


		def load_style(style: dict \| str = "color") -> StyleDict:

		if style_name in ["color"]:
		current_style = DefaultStyle().style

Change style of DAG generated in dag_drawer() #13253

Are you sure you want to change the base?

Change style of DAG generated in dag_drawer() #13253

Conversation

emilkovacev commented Oct 1, 2024 • edited Loading

Summary

Details and comments

qiskit-bot commented Oct 1, 2024

coveralls commented Oct 1, 2024 • edited Loading

Pull Request Test Coverage Report for Build 12821657503

Details

💛 - Coveralls

mtreinish left a comment

Choose a reason for hiding this comment

raynelfss left a comment

Choose a reason for hiding this comment

raynelfss Nov 4, 2024

Choose a reason for hiding this comment

raynelfss Nov 4, 2024

Choose a reason for hiding this comment

emilkovacev Nov 9, 2024

Choose a reason for hiding this comment

raynelfss Nov 4, 2024

Choose a reason for hiding this comment

raynelfss Nov 4, 2024

Choose a reason for hiding this comment

emilkovacev Nov 9, 2024 • edited Loading

Choose a reason for hiding this comment

raynelfss Nov 4, 2024

Choose a reason for hiding this comment

emilkovacev commented Oct 1, 2024 •

edited

Loading

coveralls commented Oct 1, 2024 •

edited

Loading

emilkovacev Nov 9, 2024 •

edited

Loading