Qiskit / rustworkx

A high performance Python graph library implemented in Rust.
https://www.rustworkx.org
Apache License 2.0
1.08k stars 148 forks source link

graphviz draw tooltip with special characters #750

Closed LaurentBergeron closed 3 months ago

LaurentBergeron commented 1 year ago

Information

What is the current behavior?

Adding a \n or : character to the tooltip throws this error:

Error: <stdin>: syntax error in line 4 near ']'      
Traceback (most recent call last):
  File main.py", line 6, in <module>
    graphviz_draw(
  File "venv\lib\site-packages\rustworkx\visualization\graphviz.py", line 199, in graphviz_draw
    subprocess.run(
  File "AppData\Local\Programs\Python\Python39\lib\subprocess.py", line 528, in run
    raise CalledProcessError(retcode, process.args,  
subprocess.CalledProcessError: Command '['dot', '-T', 'svg', '-o', 'graph.svg']' returned non-zero exit status 1.

What is the expected behavior?

I would like the tooltip to support special characters such as \n and :, like label does. In http://magjac.com/graphviz-visual-editor/, I'm allowed to do that with:

digraph {
  label="Graph Label"
  Node1 [tooltip="Node1 Tooltip\nlinebreak in tooltip: it's fine"]
}

Steps to reproduce the problem

import rustworkx as rx
from rustworkx.visualization import graphviz_draw

graphviz_draw(
    rx.generators.path_graph(2),
    filename="graph.svg",
    image_type="svg",
    node_attr_fn=lambda x: {"label": "the\nlabel", "tooltip": "the\ntooltip"},
)
mtreinish commented 1 year ago

I took a quick look at this, the error is we're not wrapping tooltip in double quotes so it's getting inserted as a line break and graphviz isn't able to parse it. This is the output of calling .to_dot() in your example:

graph {
0 [label="the
label", tooltip=the
tooltip];
1 [label="the
label", tooltip=the
tooltip];
0 -- 1 ;
}

The simple fix is to update: https://github.com/Qiskit/rustworkx/blob/main/src/dot_utils.rs#L89 to be label or tooltip. But I wonder if there are more fields that take strings and need to be wrapped too?

1ucian0 commented 4 months ago

For reference, the tooltip documentation https://graphviz.org/docs/attrs/tooltip/

anushkrishnav commented 4 months ago

Heyy I saw this issue at the unitaryhacks . This seems like a straightforward fix, will start on this, kindly assign this to me, will ask for doubts if something comes up.

mtreinish commented 4 months ago

This issue is participating on UnitaryHack 2024, between May 29 and June 12, 2024.

Because the nature of the event, there is no need to request to be assigned: just go ahead and PR your fix. The first/best PR can get the bounty (or it could be shared if they complement each other).

anushkrishnav commented 4 months ago

@mtreinish Got it , thank you, the PR is open , Would love to know your thoughts on if the fix is what you are looking for