Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implemented proper arrows. #2177

Closed
wants to merge 10 commits into from
Closed

Implemented proper arrows. #2177

wants to merge 10 commits into from

Conversation

paulbrodersen
Copy link
Contributor

The main challenge in this task was to ensure a consistent scaling of
draw elements (nodes, node edges, edges, and arrow heads). Matplotlib
has 3 different scales / length units (figure, axis and data) and
different draw elements are tied to different scales. In the previous
implementation draw elements with different scales were
mixed (c.f. node_size=300 but linewidth=1.), making it impossible to
design a decent layout a priori, and in particular to choose sensible
values for arrow heads and arrow head offsets to prevent occlusion by/of
nodes. In this new implementation, all draw elements are derived from
matplotlib.patches, and are hence of uniform
scale (c.f. node_size=3. and linewidth=0.5). As far as I can tell, the
only compatibility breaking change was to scrap the
draw_networkx_edges() argument 'style', as there is no equivalent in the
matplotlib.patches class.

The main challenge in this task was to ensure a consistent scaling of
draw elements (nodes, node edges, edges, and arrow heads). Matplotlib
has 3 different scales / length units (figure, axis and data) and
different draw elements are tied to different scales. In the previous
implementation draw elements with different scales were
mixed (c.f. node_size=300 but linewidth=1.), making it impossible to
design a decent layout a priori, and in particular to choose sensible
values for arrow heads and arrow head offsets to prevent occlusion by/of
nodes. In this new implementation, all draw elements are derived from
matplotlib.patches, and are hence of uniform
scale (c.f. node_size=3. and linewidth=0.5). As far as I can tell, the
only compatibility breaking change was to scrap the
draw_networkx_edges() argument 'style', as there is no equivalent in the
matplotlib.patches class.
@hagberg
Copy link
Member

hagberg commented Jul 2, 2016

Thanks for this. I couldn't get your code to work for me

$ python -c "import networkx as nx; G = nx.Graph([(1,2)]); nx.draw(G)"
Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "/mnt/hgfs/aric/Software/hagberg-networkx/networkx/drawing/nx_pylab.py", line 133, in draw
    draw_networkx(G, pos=pos, ax=ax, **kwds)
  File "/mnt/hgfs/aric/Software/hagberg-networkx/networkx/drawing/nx_pylab.py", line 272, in draw_networkx
    node_collection = draw_networkx_nodes(G, pos, **kwds)
  File "/mnt/hgfs/aric/Software/hagberg-networkx/networkx/drawing/nx_pylab.py", line 416, in draw_networkx_nodes
    position=positions[ii],
IndexError: index 2 is out of bounds for axis 0 with size 2

@paulbrodersen
Copy link
Contributor Author

The author himself! Will look into it.

@paulbrodersen
Copy link
Contributor Author

Fixed. I was so silly to assume 0-indexing of nodes...

@paulbrodersen
Copy link
Contributor Author

paulbrodersen commented Jul 3, 2016

Hey hagberg, I have implemented a convenience function to draw weighted networks (based in and in the spirit of draw()). Normally, I would file another pull request (which is why my first instinct was to remove it before pushing upstream) but the design choices really only make sense with the new version of draw_networkx_edges(). Do you mind if I append the function to this pull request?

@hagberg
Copy link
Member

hagberg commented Jul 3, 2016

I think I understand what you have done with the scaling transforms. Zooming in with this code makes everything uniformly larger. But I actually prefer the way it currently works - when you zoom in the objects stay the same size and the window frame range gets smaller. It is harder to get the arrows drawn correctly that way though.

@paulbrodersen
Copy link
Contributor Author

It's impossible to set offsets a priori such that the nodes are not occluded, it is impossible to design anything without fixing the axis sizes because you have no idea how the different elements will play together. If you want to automate your figure generation than these things are bad news.

@hagberg
Copy link
Member

hagberg commented Jul 3, 2016

It's definitely possible to design something that scales dynamically - we have that now. It isn't so easy to keep the nodes and edges from overlapping or crossing but that is more a layout issue than a drawing issue.
Last time I checked in with the matplotlib developers (about a year ago) there were some missing pieces to make it easier to draw arrows (I forget the details, related to arrow collections and transforms). You can even make curved edges, edges that connect to the nearest polygon corner, etc.

@paulbrodersen
Copy link
Contributor Author

Thought I would add some visual examples -- after all, a picture speaks more than a 1000 words.

Example output for random, unweighted network:
figure_1

Example output for random, weighted network:
figure_5

@paulbrodersen
Copy link
Contributor Author

paulbrodersen commented Jul 3, 2016

(Sorry, I was making example figures and didn't see your comment before I posted them.)

Ultimately, your node positions will always be in data coordinates. Would you not want your draw elements in the same coordinate system to prevent surprises? I see the appeal to be able to zoom in and have the markers changing dynamically. However, that is the same reason you cannot design a figure from the ground up at the moment because you don't actually know how big the draw elements will be because it all depends on the axis size.

@hagberg
Copy link
Member

hagberg commented Jul 4, 2016

I'm not an expert in matplotlib or graph drawing. I have learned that drawing graphs is tricky and that there are many different approaches and goals for drawings. I doubt that any one system will satisfy everyone or even most. That is one of the reasons I have proposed moving the matplotlib drawing code out of the networkx library. That reason isn't stated in #1325 where there are some other thoughts and ideas but doing so would allow multiple drawing systems with different approaches for different goals. I'd also like to see the design of the current drawing package redone to be simpler to use and to understand.

The reason I bring this up here is that this PR, though it doesn't modify the drawing API much, substantially changes the output and might be better as a completely separate drawing function, e.g. aimed more at publication-quality small graph drawings than interactive exploration of the data.

@paulbrodersen
Copy link
Contributor Author

That seems reasonable. Any suggestions on how to proceed?

@pwichmann
Copy link

Wait. Does networkx now have the capability to draw arrows?

@dschult
Copy link
Member

dschult commented Jun 2, 2018

Arrows were included in #2760

@dschult dschult closed this Jun 2, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

4 participants