Matplotlib multigraph edges (undirected) filled

[iosonofabio]

Fixing #731.

I had it fixed and then took out a test to make it directed, and of course that failed to detect a regression. Now I fixed the bug and added a test for it, so it should be ok.

Notice I'm not really happy about the general solution, which involves retracing the edge twice, but mpl can be a little finnicky at times. I feel like from the lessons learned in our latest iteration of the plotting backend I could perhaps make one or two big PRs in mpl and improve the upstream behaviour. Anyway, a story for another day.

Ready to merge if it passes CI (it passes local tests on my machine already).

[ntamas]

So, please let me know if I understand this correctly:

• We have a single EdgeCollection artist that is responsible for drawing all the edges. MatplotlibEdgeDrawer isn't really used for anything, right? (Would it be better to remove it altogether?)
• This artist is drawn with facecolor set to a common edge color (does this mean that we cannot set edge colors separately in the Matplotlib backend now?) to ensure that the arrowheads are filled with the same color as the one used for drawing the lines.
• Matplotlib implicitly fills each path in the EdgeCollection due to the facecolor being set to a non-empty color. For straight edges this doesn't matter because closing the path still yields a degenerate triangle only so it is drawn as a line. For directed curved edges this doesn't matter either because we already took care of retracing our steps through the curve back to the starting point. However, we did not do that for undirected curved edges, hence the bug.

Is my understanding correct? If it is, then I'm okay to merge this as a hotfix although I don't really like the trick with essentially drawing the edge twice. I wonder whether it would be possible to talk to the Matplotlib authors and mention this as a limitation. I see that there is a STOP opcode for paths, which the documentation claims is currently ignored, but I was wondering whether this should be used as an indicator for "open" paths. Then we would not need to retrace our steps back to the start when drawing an undirected edge.

Another alternative solution for the future would be to draw the edges with one artist and the arrowheads with another artist. This still wouldn't solve the limitation of not being able to use different colors for different edges.

All in all, I don't know how much time we should spend on tidying up the Matplotlib drawing backend in the current version. I think that maybe we should focus our efforts on designing a pluggable drawing backend API properly for the "new" Python interface with ctypes, based on the lessons learned here. At some point I'd appreciate your thoughts on how that API should look like, although I'm not quite there yet with the new interface so it's definitely not for this year but maybe for the next round.

[iosonofabio]

Hi @ntamas

Correct. It's dirty but revealed a bunch of limitations in mol and hidden requirements from our end. I agree that we would start focusing on the new prototype, which is why I'm fine with a dirty design for this old version.

I believe that you can change the color on a per edge basis but that's not central to the argument. Meeting up with mpl folks would be the logical next step. I can inquire with them to test the waters.

[ntamas]

Okay, happy to merge this as is then.

[iosonofabio]

Just to contextualize for future reference.

The reason I rely on collections is that mpl collections have a mechanism to draw each element (e.g. a node marker) in figure points (or other units) and then shift it into position using data units (i.e. the layout coordinates). They roughly call it OffsetTransform.

That does not exist for single elements. For instance, if we use a Circle, we probably want the center coordinates in data units but the radius in figure units, however the mpl constructor for Circle only allows one type of unit for the whole object. They call this one Transform (no "offset").

For edges it's even more tricky because we want to draw the line between two data coordinates minus an offset (the marker size) which is in figure points. This needs to be recomputed when you zoom in or out.

I am going to reach out to mpl folks and ask them for a zoom meeting.