Add support for labelling of lines #20
Conversation
AnjoMan
changed the title
tests/test_asciigraf.py
Outdated
| """) | ||
|
|
||
| assert ("A", "B") in graph.edges() | ||
| assert ("A", "(ABC)") not in graph.edges() |
There was a problem hiding this comment.
what's the reason for checking that a link between A and (ABC) does not exist?
| for offset, _ in enumerate(node_label) | ||
| } | ||
| if node_label.startswith("(") and node_label.endswith(")"): | ||
| label_value = node_label[1:-1] |
There was a problem hiding this comment.
was pleasantly surprised to find that this works even if node_label == '()' (at least on python 3.6)
There was a problem hiding this comment.
Yeah, you should just get an empty string on the edge attribute.
There was a problem hiding this comment.
Also 2.x
Python 2.7.10 (default, Feb 7 2017, 00:08:15)
[GCC 4.2.1 Compatible Apple LLVM 8.0.0 (clang-800.0.34)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> "()"[1:-1]
''
There was a problem hiding this comment.
It makes sense it's the substring with the same starting and ending index in the string.
AnjoMan
changed the title
|
|
||
| # nodes = node_chars | ||
| # we'll treat edge labels (e.g. "(my_label)") as consecutive "-" edge chars |
There was a problem hiding this comment.
I don't agree because renaming it means when we rename the label, we change the length, though I understand it's probably easier for other reasons like it makes it simpler to align with other lines and points on the graph.
There was a problem hiding this comment.
So basically I agree but I don't 100% love it. It's pragmatic and simple which is good.
There was a problem hiding this comment.
We could go back at the end and modify the lengths of all the edges by subtracting the length of the label.
There was a problem hiding this comment.
Thinking about it more, it also means the user is forced to lengthen lines that connect to the the graph so it might be best to keep the current behaviour, at least everything will be of the same length in relative terms.
|
|
||
| # nodes = node_chars | ||
| # we'll treat edge labels (e.g. "(my_label)") as consecutive "-" edge chars |
There was a problem hiding this comment.
So basically I agree but I don't 100% love it. It's pragmatic and simple which is good.
* support line labels using brackets * test them * don't add label nodes to the graph
tests/test_asciigraf.py
Outdated
| | | ||
| | | ||
| D---(string)----E | ||
|
|
|
|
||
| # nodes = node_chars | ||
| # we'll treat edge labels (e.g. "(my_label)") as consecutive "-" edge chars |
|
|
||
| # nodes = node_chars | ||
| # we'll treat edge labels (e.g. "(my_label)") as consecutive "-" edge chars |
There was a problem hiding this comment.
Thinking about it more, it also means the user is forced to lengthen lines that connect to the the graph so it might be best to keep the current behaviour, at least everything will be of the same length in relative terms.
| n0-------------n1----------n2 | ||
| | | ||
| | (3) | ||
| | <3> |
There was a problem hiding this comment.
Can we add a test that covers mixing (labels) and <lengths> ?
A----(my_label)-----Bin edges