graph vertices cut off #9211

jasongrout · 2010-06-11T06:48:32Z

Though #7299 helped, graph vertices are still cut off.

We can turn off clipping of the matplotlib scatterplot and add some bbox_extra_artists to the savefig which takes those artists into account when calculating the bounding box (when bbox_inches='tight').

First apply attachment: trac-9211-fix_cut_vertices_in_graphs.patch and then apply attachment: trac_9211_digraph_clipping.patch to $SAGE_ROOT/devel/sage. Finally apply attachment: trac_9211-fix_doctests.patch to $SAGE_ROOT/devel/sage.

CC: @rbeezer @kcrisman @kini

Component: graph theory

Author: Jason Grout

Reviewer: Punarbasu Purkayastha

Merged: sage-4.8.alpha1

Issue created by migration from https://trac.sagemath.org/ticket/9211

The text was updated successfully, but these errors were encountered:

jasongrout · 2010-06-11T06:49:44Z

comment:1

Here is a good example showing the problem (still):

a=matrix([[1,2],[3,4]])

var('x y z w')
b=matrix([[x,y],[z,w]])

a.set_immutable()
b.set_immutable()

K=DiGraph({a:[b]})
show(K, vertex_size=800)

jasongrout · 2010-06-12T21:22:33Z

Attachment: trac-9211-graph-vertices-cut.patch.gz

jasongrout · 2010-06-12T21:24:40Z

comment:2

This patch depends on the matplotlib spkg at #9221. I don't know if the patch is very elegant. I'll trust the reviewer's comments on that. The problem is that it lets the scatter plot vertices extend beyond the frame (relying on the user adjusting the axes_pad option to extend the frame).

jasongrout · 2010-06-12T21:25:02Z

Author: Jason Grout

jasongrout · 2010-06-12T21:25:02Z

Work Issues: depends on #9221

jasongrout · 2010-06-12T22:34:03Z

comment:4

The patch doesn't work for multiple-edge graphs (they are drawn completely differently than graphs without multiple edges). In fact, we probably ought to draw all graphs using CircleCollection objects rather than scatter_plot or Circles.

jasongrout · 2010-08-28T14:06:25Z

comment:5

Here's how to draw circles like the ones in scatter plot (where the size doesn't change as you make the figure smaller or bigger: use a CircleCollection argument and use different transforms to specify location and size coordinates. See http://sourceforge.net/mailarchive/forum.php?thread_name=AANLkTim6ZnGiOV1RTRBz0uBMF5y67xyrDp%2BkVSCBtkrB%40mail.gmail.com&forum_name=matplotlib-users

jasongrout · 2010-08-28T20:02:56Z

comment:6

See also http://groups.google.com/group/networkx-discuss/browse_thread/thread/b95fda9813fae67d for code/ideas for rewriting the graph drawing.

jasongrout · 2011-03-23T02:41:22Z

comment:7

I think this patch might take just a little to clean up. Most of the patch is just adding comments to explain the code or reformatting long lines. The big key is the clip=False arguments and the code to add things to the bbox_extra_artists.

I think the patch might just need some doctests and testing. The other comments I put in above can be moved to another ticket that does a bigger revamp of graph drawing.

kcrisman · 2011-03-23T12:19:33Z

Changed work issues from depends on #9221 to none

ppurka · 2011-10-11T09:28:52Z

( Fix cut off vertices in graphs ) Apply to devel/sage/

ppurka · 2011-10-11T09:45:33Z

Attachment: trac-9211-fix_cut_vertices_in_graphs.patch.gz

Attachment: trac-9211-add_padding_to_graphs.patch.gz

( Add additional padding to graphs ) Apply to devel/sage

ppurka · 2011-10-11T09:46:15Z

Output with only the earlier patch (modified for 4.7.2)

ppurka · 2011-10-11T09:46:37Z

Attachment: with-earlier-patch-modified-for-4.7.2.png

Output with patch for additional padding

ppurka · 2011-10-11T09:57:32Z

comment:11

Attachment: with-additional-padding.png

Added two patches. The first one is essentially the patch by jason redone to work with 4.7.2_alpha3.

The second patch adds extra padding to all graphs. The two pngs attached are the output of

M = matrix(RDF, [[1/3, 1/3, 1/3], [0, 1/4, 3/4], [1/2, 1/4, 1/4]])
G=DiGraph(M, format='weighted_adjacency_matrix')
G.show(vertex_size=800, edge_labels=True)

The file attachment: with-additional-padding.png is the result of both the patches being applied.

Graphs currently look really bad without either of the two patches. The second patch is not a panacea for all cut off graph plots since it doesn't fix all cases. But my hope is that it fixes most commonly used cases.

jasongrout · 2011-10-11T13:14:32Z

comment:12

I'm curious why you need the extra padding patch. Are vertices still cut off using just the first patch?

ppurka · 2011-10-11T13:38:16Z

comment:13

Replying to @jasongrout:

I'm curious why you need the extra padding patch. Are vertices still cut off using just the first patch?

Yes. Check the plot after the first patch against the the plot after first+second patch

ppurka · 2011-10-11T14:45:32Z

comment:16

Replying to @jasongrout:

Can you try applying just trac-9211-fix_cut_vertices_in_graphs.patch and trac_9211_digraph_clipping.patch ?

Excellent! Thanks. This does fix the problem for digraphs. Much better than adding random paddings.

jasongrout · 2011-10-11T15:32:00Z

comment:19

Okay, did you run long tests on Sage? The new version of my first patch seems okay to me, so if you positively review the patches, I think we are good to go, assuming that you've run long tests and there are no doctest errors.

ppurka · 2011-10-11T16:02:20Z

comment:20

Replying to @jasongrout:

Okay, did you run long tests on Sage? The new version of my first patch seems okay to me, so if you positively review the patches, I think we are good to go, assuming that you've run long tests and there are no doctest errors.

I started the long test (make ptestlong) over an hour ago. I guess there's 3 more hours to go :)

ppurka · 2011-10-12T03:37:41Z

comment:21

A couple of doctests failed. Only in the directory plot/ (and in the files changed by this patch + base.pyx). Will investigate more once I am physically near that machine where the doctest was run.

kcrisman · 2011-10-12T03:53:32Z

comment:22

A couple of doctests failed. Only in the directory plot/ (and in the files changed by this patch + base.pyx). Will investigate more once I am physically near that machine where the doctest was run.

Just FYI, base.pyx often fails on Mac if you don't have libjpeg or libtiff in the right place - see #7344 and #7345. If the errors look like

    ImportError: The _imaging C module is not installed

you can ignore them, they would be unrelated.

ppurka · 2011-10-12T08:52:05Z

comment:23

I am running it on x86_64 Linux. The errors were because of the additional 'clip' option introduced in the patches. Attaching a fix for the doctests.

kini · 2011-10-12T08:56:57Z

comment:24

I wonder why you are concatenating lines in the doctest output - our docstrings are theoretically supposed to be 72 (or 79) characters wide, if I'm not mistaken, which is why they the current test results are broken into multiple lines. The doctester doesn't care about purely whitespace differences between the expected and received output.

ppurka · 2011-10-12T08:58:20Z

comment:25

Replying to @kini:

I wonder why you are concatenating lines in the doctest output - our docstrings are theoretically supposed to be 72 (or 79) characters wide, if I'm not mistaken, which is why they the current test results are broken into multiple lines. The doctester doesn't care about purely whitespace differences between the expected and received output.

Simply copied the output from "Expected output". Will attached a space-ified fix. :)

kini · 2011-10-12T09:06:04Z

comment:26

I wonder if you are aware of sage -fixdoctests, which does basically exactly what you are doing... :) BTW I have the same comment for attachment: trac-9211-add_padding_to_graphs.patch.

ppurka · 2011-10-12T09:09:57Z

Attachment: trac_9211-fix_doctests.patch.gz

Fix doctests for 3d plots (re-uploaded)

ppurka · 2011-10-12T09:11:58Z

comment:27

Uploaded new patch.

Didn't know about -fixdoctests (it wasn't too painful to fix because of vim)

Forget about the adding padding patch. Jason did the real fix. :)

ppurka · 2011-10-12T09:14:27Z

comment:28

Previous failures were:

----------------------------------------------------------------------

The following tests failed:

	sage -t  -long -force_lib devel/sage/sage/plot/scatter_plot.py # 1 doctests failed
	sage -t  -long -force_lib devel/sage/sage/plot/text.py # 5 doctests failed
	sage -t  -long -force_lib devel/sage/sage/plot/circle.py # 8 doctests failed
	sage -t  -long -force_lib devel/sage/sage/plot/plot3d/base.pyx # 5 doctests failed
----------------------------------------------------------------------
Total time for all tests: 8844.0 seconds

New doctest on devel/sage/sage/plot/ passes all doctests:

~/Installations/sage-4.7.2> ./sage -t -long devel/sage/sage/plot
...
----------------------------------------------------------------------
All tests passed!
Total time for all tests: 520.6 seconds

So positive review from my side.

ppurka · 2011-10-12T09:14:27Z

Reviewer: Punarbasu Purkayastha

jasongrout · 2011-10-12T15:06:45Z

comment:30

Your changes were a little more than just fixing whitespace, so let someone look at it to review your last patch.

Sorry---you're right about those doctests. I shouldn't have been so sloppy as to not fix those doctests before indicating it needed to be reviewed.

jasongrout · 2011-11-04T17:53:54Z

comment:31

Yep, I confirm that normal tests in graphs/ and plot/ pass. Positive review for the fix doctests patch. Good job!

jdemeyer · 2011-11-07T10:10:28Z

Merged: sage-4.8.alpha1

jasongrout added this to the sage-4.8 milestone Jun 11, 2010

jasongrout added c: graph theory labels Jun 11, 2010

jasongrout assigned jasongrout, sagetrac-mvngu , nathanncohen and rlmill Jun 11, 2010

jasongrout added the s: needs review label Jun 12, 2010

jasongrout added s: needs work and removed s: needs review labels Jun 12, 2010

ppurka added s: needs review and removed s: needs work labels Oct 11, 2011

This comment has been minimized.

Sign in to view

ppurka added s: positive review and removed s: needs review labels Oct 12, 2011

jasongrout added s: needs work and removed s: positive review labels Oct 12, 2011

jasongrout added s: needs review and removed s: needs work labels Oct 12, 2011

jasongrout added s: positive review and removed s: needs review labels Nov 4, 2011

jdemeyer removed the s: positive review label Nov 7, 2011

jdemeyer closed this as completed Nov 7, 2011

williamstein mentioned this issue Jan 5, 2012

graph plotting -- ploting of graphs (networks) is somehow messed up/scaled wrong/cropped wrong since it doesn't work with graphics_array #5938

Closed

graph vertices cut off #9211

graph vertices cut off #9211

Comments

jasongrout commented Jun 11, 2010

jasongrout commented Jun 11, 2010

jasongrout commented Jun 12, 2010

jasongrout commented Jun 12, 2010

jasongrout commented Jun 12, 2010

jasongrout commented Jun 12, 2010

jasongrout commented Jun 12, 2010

jasongrout commented Aug 28, 2010

jasongrout commented Aug 28, 2010

jasongrout commented Mar 23, 2011

kcrisman commented Mar 23, 2011

ppurka commented Oct 11, 2011

ppurka commented Oct 11, 2011

ppurka commented Oct 11, 2011

ppurka commented Oct 11, 2011

ppurka commented Oct 11, 2011

jasongrout commented Oct 11, 2011

ppurka commented Oct 11, 2011

ppurka commented Oct 11, 2011

This comment has been minimized.

This comment has been minimized.

jasongrout commented Oct 11, 2011

ppurka commented Oct 11, 2011

ppurka commented Oct 12, 2011

kcrisman commented Oct 12, 2011

This comment has been minimized.

ppurka commented Oct 12, 2011

kini commented Oct 12, 2011

ppurka commented Oct 12, 2011

kini commented Oct 12, 2011

ppurka commented Oct 12, 2011

ppurka commented Oct 12, 2011

ppurka commented Oct 12, 2011

ppurka commented Oct 12, 2011

jasongrout commented Oct 12, 2011

jasongrout commented Nov 4, 2011

jdemeyer commented Nov 7, 2011