Opened 5 years ago
Last modified 5 years ago
#18046 needs_work enhancement
Graphs with multiedges and latex
Reported by:  jclaas  Owned by:  

Priority:  major  Milestone:  sage6.6 
Component:  graph theory  Keywords:  graphtheory, multiedges, latex, sd66 
Cc:  ncohen, nthiery, tscrim, aschilling  Merged in:  
Authors:  Jacob Laas  Reviewers:  
Report Upstream:  N/A  Work issues:  
Branch:  u/jclaas/graphs_with_multiedges_and_latex (Commits)  Commit:  d2ebe228caee2da94ecee214bf7a8f52004228be 
Dependencies:  Stopgaps: 
Description (last modified by )
It was discovered that the LaTeX module for graphs raises the NotImplementedError? for graphs containing multiple edges (multiedges=True
). The intention of this ticket is to incorporate the fixes needed to typeset a graph with latex.
Change History (10)
comment:1 Changed 5 years ago by
 Branch set to u/jclaas/graphs_with_multiedges_and_latex
 Cc ncohen nthiery tscrim aschilling added
 Component changed from PLEASE CHANGE to graph theory
 Description modified (diff)
 Keywords graphtheory multiedges latex added
 Status changed from new to needs_info
 Type changed from PLEASE CHANGE to enhancement
comment:2 Changed 5 years ago by
comment:3 Changed 5 years ago by
 Commit set to b025f2ad103c5596edde303e6bcf983ecea4d0e2
Branch pushed to git repo; I updated commit sha1. New commits:
b025f2a  all features from local hack have been added, plus a warning about edge customizations

comment:4 Changed 5 years ago by
I've just pushed a commit containing updated code so that graphs with multiedges can be texified, but there's a particular case that still causes issue: when a set of multiedges contain labels, their customizations do not match up correctly.
My first guess was that this due to the nature of using a dict_of_dicts to track all edge/vertex properties, which python then gets the order of hashable items mixed up.
See below for a test case illustrating the issue:
mygraph = DiGraph(multiedges=True) mygraph.add_edge('A','B','thinnest') mygraph.add_edge('A','B','blah') mygraph.add_edge('A','B','thickest') mygraph.add_edge('A','C','blah') mygraph.add_edge('B','C','blah') mygraph.add_edge('C','B','blah') mygraph.set_latex_options( units='in', graphic_size=(6,6), edge_thickness=0.01, edge_labels=True, edge_thicknesses={('A','B'):[0.01,0.025,0.05]}, ) print latex(mygraph) view(latex(mygraph))
I've specified a list of thicknesses for the multiedges 'A' > 'B'. Their labels were defined in a particular order, and the thicknesses are meant to match this order, but they don't.. any ideas?
comment:5 Changed 5 years ago by
 Status changed from needs_info to needs_review
comment:6 Changed 5 years ago by
 Commit changed from b025f2ad103c5596edde303e6bcf983ecea4d0e2 to b3721b4fd42959a87ee415643a44776d2e8d83c2
Branch pushed to git repo; I updated commit sha1. New commits:
b3721b4  replaced new variables with long names, replaced stopgap with NotImplementedError, fixed typo in internal type check for edge customizations

comment:7 Changed 5 years ago by
 Keywords sd66 added
 Status changed from needs_review to needs_work
Hello,
Several other remarks:
 These days I have been told that we should use
isinstance(x,list)
instead oftype(x) in [list]
. Note that the following works:
sage: isinstance([],(list,dict)) True sage: isinstance({},(list,dict)) True sage: isinstance(5,(list,dict)) True
 Now, this file looks rather messy and you apparently tried to get your code to
'fit in the local style'. It seems that they define a variable named
number_types
that you could use.
et
> no short name please.
 This line contains both
and
andor
. Could you add parentheses where they belong ?
if self._graph.is_directed() or self._graph.allows_multiple_edges() and not loop:
 Could you write the proper documentation (in the INPUT section of the docstring) for the parameter
edge_thickness
in case of multiple edges?
 Your error message is misleading: it is not that multiple customization of
multiedges is impossible, it is that only
thickness
can be set. At least, this is what your condition tests.
 What is the difference between
>
andpost
in TikZ? Why did you make that change?
 Is there any reason why you call
self._graph.edge_label(edge[0],edge[1])
? That value should already be equal toe[2]
, shouldn't it?
Nathann
comment:8 Changed 5 years ago by
Thanks for your feedback. Sorry for missing some of the style rules, but some of the issues you raise are actually left over from previous work. Regardless, I've tried to fix them the best I can.
Also, I'm happy to report that I've gotten custom edge_thicknesses
to work. It turned out that I just had to reverse the value of multiedge_index. I didn't dive too deep to find out why, as there's something like 27k lines of code I might have to search through.
 These days I have been told that we should use
isinstance(x,list)
instead oftype(x)
in [list].
Fixed.
 Now, this file looks rather messy and you apparently tried to get your code to 'fit in the local style'. It seems that they define a variable named
number_types
that you could use.
Fixed.
et
> no short name please.
Fixed.
 This line contains both
and
andor
. Could you add parentheses where they belong ?
Fixed.
 Could you write the proper documentation (in the INPUT section of the docstring) for the parameter
edge_thickness
in case of multiple edges?
Fixed.
 Your error message is misleading: it is not that multiple customization of multiedges is impossible, it is that only
thickness
can be set. At least, this is what your condition tests.
I've edited the message to read "Edgespecific colors are not available for multiedges.", so that it only refers to the case of edge_colors, which I wasn't up to the task of fixing immediately. The color issue seems like a lot of work, because each vertex/edge color is specified via unique \definecolor{}{rgb}{
} latex definitions, instead of just passing colors as strings. From what I can tell from the documentation (though I don't understand French), tkzgraph understands colors as strings, so it seems like the entire set of \definecolor{}{rgb}{
} definitions are unnecessary. Perhaps one should open a ticket just for updating this feature? I could see an update to this being more useful for my own purposes, but it also just seems better to just have colors specified within each edge's texstring.
 What is the difference between
>
andpost
in TikZ? Why did you make that change?
I don't know what post
is, but I replaced it with >
because post
doesn't respect line widths and it's not in the most recent documentation (https://www.ctan.org/pkg/tkzgraph).
 Is there any reason why you call
self._graph.edge_label(edge[0],edge[1])
? That value should already be equal toe[2]
, shouldn't it?
Hmm that wasn't my doing, but I did find out that attempting to refer to e[2]
absolutely doesn't maintain the order in which the edge labels are called, which I find very bizarre. Leaving it asis will at least ensure that my fix for edge_thicknesses
works properly.
comment:9 Changed 5 years ago by
 Commit changed from b3721b4fd42959a87ee415643a44776d2e8d83c2 to d2ebe228caee2da94ecee214bf7a8f52004228be
Branch pushed to git repo; I updated commit sha1. New commits:
d2ebe22   fixed edge_thicknesses and many styling issues

comment:10 Changed 5 years ago by
Helloooooooooooo,
Several coments:
 Could you add to the doctests an example of a graph plot which uses the new feature you impplemented?
if isinstance(x, list) and (not isinstance(x[0], number_types) or not x[0] >= 0.0):
This code only tests if the *first* element of the list is a positive. You should probably replace
x[0] >= 0.0
withall(xx>=0 for xx in x)
 The previous test appears so often that it would be better to define a
is_positive_number=lambda x:isinstance(x, number_types) and x >= 0.0
and use it everywhere.
 In this block of code
reverse_index = len(edge_thickness)  (multiedge_index+1) edge_thickness = edge_thickness[reverse_index]
You should probably use the following Python tricksage: a=[1,2,3,4,5,6] sage: a[1] 6 sage: a[4] 3 sage: a[5] 2 sage: a[6] 1
Short of this the code looks good. Thanks,
Nathann
I should note that I've already incorporated most of the necessary fixes into a local copy of
sage/graphs/graph_latex.py
and will soon post here a patch for the file, so that I can get some feedback..