Skip to content

Commit

Permalink
Fix dot file generation
Browse files Browse the repository at this point in the history
This commit fixes a regression introduced in Qiskit#1203. That PR was
attempting to fix missing character escaping in some string fields
but it was doing so too eagerly. This would result in invalid dot
files being produced for users upgrading from rustworkx < 0.15.0 that
were wrapping strings in quotes as needed previously. For example if you
were setting `'color="#aaaaaa"'` previously before this would become
`color="\"#aaaaaa\""` after Qiskit#1203. In order to quickly release a 0.15.1
this commit reverts the dot generation component of Qiskit#1203 but updates
the code to wrap tooltip in addition to label which was what the
original bug reported. In 0.16.0 we should investigate adding a flag to
control the escaping behavior of the function to either decide to wrap
values in quotes or not.
  • Loading branch information
mtreinish committed Jun 28, 2024
1 parent 0d648b2 commit 2292b1f
Show file tree
Hide file tree
Showing 5 changed files with 90 additions and 20 deletions.
21 changes: 21 additions & 0 deletions releasenotes/notes/fix-graphviz-escaping-39a2d1cebb586eca.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
---
fixes:
- |
Fixed an issue in the :func:`~.graphviz_draw`, :meth:`.PyGraph.to_dot`, and
:meth:`.PyDiGraph.to_dot` which was incorrectly escaping strings when
upgrading to 0.15.0. In earlier versions of rustworkx if you manually
placed quotes in a string for an attr callback get that to pass through to
the output dot file this was incorrectly being converted in rustworkx 0.15.0
to duplicate the quotes and escape them. For example, if you defined a
callback like::
def color_node(_node):
return {
"color": '"#422952"'
}
to set the color attribute in the output dot file with the string
`"#422952"` (with the quotes) this was incorrectly being converted to
`\"#422952\"`. This no longer occurs, in rustworkx 0.16.0 there will likely
be additional options exposed in :func:`~.graphviz_draw`,
:meth:`.PyGraph.to_dot`, and :meth:`.PyDiGraph.to_dot` to expose further
options around this.
16 changes: 8 additions & 8 deletions src/dot_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,8 @@ where
Ok(())
}

static ATTRS_TO_ESCAPE: [&str; 2] = ["label", "tooltip"];

/// Convert an attr map to an output string
fn attr_map_to_string<'a>(
py: Python,
Expand All @@ -85,15 +87,13 @@ fn attr_map_to_string<'a>(
let attr_string = attrs
.iter()
.map(|(key, value)| {
let escaped_value = serde_json::to_string(value).map_err(|_err| {
pyo3::exceptions::PyValueError::new_err("could not escape character")
})?;
let escaped_value = &escaped_value.get(1..escaped_value.len() - 1).ok_or(
pyo3::exceptions::PyValueError::new_err("could not escape character"),
)?;
Ok(format!("{}=\"{}\"", key, escaped_value))
if ATTRS_TO_ESCAPE.contains(&key.as_str()) {
format!("{}=\"{}\"", key, value)
} else {
format!("{}={}", key, value)
}
})
.collect::<PyResult<Vec<String>>>()?
.collect::<Vec<String>>()
.join(", ");
Ok(format!("[{}]", attr_string))
}
6 changes: 3 additions & 3 deletions tests/digraph/test_dot.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,9 @@ def test_digraph_to_dot_to_file(self):
)
graph.add_edge(0, 1, dict(label="1", name="1"))
expected = (
'digraph {\n0 [color="black", fillcolor="green", label="a", '
'style="filled"];\n1 [color="black", fillcolor="red", label="a", '
'style="filled"];\n0 -> 1 [label="1", name="1"];\n}\n'
'digraph {\n0 [color=black, fillcolor=green, label="a", '
'style=filled];\n1 [color=black, fillcolor=red, label="a", '
'style=filled];\n0 -> 1 [label="1", name=1];\n}\n'
)
res = graph.to_dot(lambda node: node, lambda edge: edge, filename=self.path)
self.addCleanup(os.remove, self.path)
Expand Down
18 changes: 9 additions & 9 deletions tests/graph/test_dot.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,9 @@ def test_graph_to_dot(self):
)
graph.add_edge(0, 1, dict(label="1", name="1"))
expected = (
'graph {\n0 [color="black", fillcolor="green", label="a", style="filled"'
'];\n1 [color="black", fillcolor="red", label="a", style="filled"];'
'\n0 -- 1 [label="1", name="1"];\n}\n'
'graph {\n0 [color=black, fillcolor=green, label="a", style=filled'
'];\n1 [color=black, fillcolor=red, label="a", style=filled];'
'\n0 -- 1 [label="1", name=1];\n}\n'
)
res = graph.to_dot(lambda node: node, lambda edge: edge)
self.assertEqual(expected, res)
Expand All @@ -70,9 +70,9 @@ def test_digraph_to_dot(self):
)
graph.add_edge(0, 1, dict(label="1", name="1"))
expected = (
'digraph {\n0 [color="black", fillcolor="green", label="a", '
'style="filled"];\n1 [color="black", fillcolor="red", label="a", '
'style="filled"];\n0 -> 1 [label="1", name="1"];\n}\n'
'digraph {\n0 [color=black, fillcolor=green, label="a", '
'style=filled];\n1 [color=black, fillcolor=red, label="a", '
'style=filled];\n0 -> 1 [label="1", name=1];\n}\n'
)
res = graph.to_dot(lambda node: node, lambda edge: edge)
self.assertEqual(expected, res)
Expand All @@ -97,9 +97,9 @@ def test_graph_to_dot_to_file(self):
)
graph.add_edge(0, 1, dict(label="1", name="1"))
expected = (
'graph {\n0 [color="black", fillcolor="green", label="a", '
'style="filled"];\n1 [color="black", fillcolor="red", label="a", '
'style="filled"];\n0 -- 1 [label="1", name="1"];\n}\n'
'graph {\n0 [color=black, fillcolor=green, label="a", '
'style=filled];\n1 [color=black, fillcolor=red, label="a", '
'style=filled];\n0 -- 1 [label="1", name=1];\n}\n'
)
res = graph.to_dot(lambda node: node, lambda edge: edge, filename=self.path)
self.addCleanup(os.remove, self.path)
Expand Down
49 changes: 49 additions & 0 deletions tests/visualization/test_graphviz.py
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,55 @@ def test_filename(self):
if not SAVE_IMAGES:
self.addCleanup(os.remove, "test_graphviz_filename.svg")

def test_qiskit_style_visualization(self):
"""This test is to test visualizations like qiskit performs which regressed in 0.15.0."""
graph = rustworkx.generators.cycle_graph(4)
colors = ["#422952", "#492d58", "#4f305c", "#5e3767"]
edge_colors = ["#4d2f5b", "#693d6f", "#995a88", "#382449"]
pos = [(0, 0), (0, 1), (1, 0), (1, 1)]
for node in graph.node_indices():
graph[node] = node

for edge in graph.edge_indices():
graph.update_edge_by_index(edge, edge)

def color_node(node):
out_dict = {
"label": str(node),
"color": f'"{colors[node]}"',
"pos": f'"{pos[node][0]}, {pos[node][1]}"',
"fontname": '"DejaVu Sans"',
"pin": "True",
"shape": "circle",
"style": "filled",
"fillcolor": f'"{colors[node]}"',
"fontcolor": "white",
"fontsize": "10",
"height": "0.322",
"fixedsize": "True",
}
return out_dict

def color_edge(edge):
out_dict = {
"color": f'"{edge_colors[edge]}"',
"fillcolor": f'"{edge_colors[edge]}"',
"penwidth": str(5),
}
return out_dict

graphviz_draw(
graph,
node_attr_fn=color_node,
edge_attr_fn=color_edge,
filename="test_qiskit_style_visualization.png",
image_type="png",
method="neato",
)
self.assertTrue(os.path.isfile("test_qiskit_style_visualization.png"))
if not SAVE_IMAGES:
self.addCleanup(os.remove, "test_qiskit_style_visualization.png")

def test_escape_sequences(self):
# Create a simple graph
graph = rustworkx.generators.path_graph(2)
Expand Down

0 comments on commit 2292b1f

Please sign in to comment.