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

Enable thickness-option in graph plotting #20035

Closed
jm58660 mannequin opened this issue Feb 11, 2016 · 24 comments
Closed

Enable thickness-option in graph plotting #20035

jm58660 mannequin opened this issue Feb 11, 2016 · 24 comments

Comments

@jm58660
Copy link
Mannequin

jm58660 mannequin commented Feb 11, 2016

This trivial patch allows thickness-option in graph plotting. It seems to have forgotten from the option list.

To test try for example

G = graphs.CompleteGraph(4)
G.allow_loops(True)
G.allow_multiple_edges(True)
G.set_edge_label(1,3,'foo')
G.set_edge_label(1,0,'foo')
G.add_edge(1,1)
G.add_edge(1,3)
G.show(color_by_label=True, edge_thickness=2.5)

CC: @anneschilling @fchapoton @slel

Component: graphics

Author: Jori Mäntysalo

Branch: 674361a

Reviewer: Paul Masson

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

@jm58660 jm58660 mannequin added this to the sage-7.1 milestone Feb 11, 2016
@jm58660 jm58660 mannequin added c: graphics labels Feb 11, 2016
@jm58660
Copy link
Mannequin Author

jm58660 mannequin commented Feb 11, 2016

@jm58660
Copy link
Mannequin Author

jm58660 mannequin commented Feb 11, 2016

New commits:

908f9b9Enable thickness-option in graph plotting.

@jm58660
Copy link
Mannequin Author

jm58660 mannequin commented Feb 11, 2016

Commit: 908f9b9

@jm58660 jm58660 mannequin added the s: needs review label Feb 11, 2016
@nathanncohen
Copy link
Mannequin

nathanncohen mannequin commented Feb 11, 2016

comment:3

I am wondering about the name: vertex_colors, edge_colors, why not edge_thickness to go with them?

Nathann

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 11, 2016

Branch pushed to git repo; I updated commit sha1. New commits:

55ab51aFrom 'thickness' to 'edge_thickness'.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 11, 2016

Changed commit from 908f9b9 to 55ab51a

@jm58660
Copy link
Mannequin Author

jm58660 mannequin commented Feb 11, 2016

comment:5

Replying to @nathanncohen:

I am wondering about the name: vertex_colors, edge_colors, why not edge_thickness to go with them?

True. Changed.

@jm58660

This comment has been minimized.

@nathanncohen
Copy link
Mannequin

nathanncohen mannequin commented Feb 11, 2016

comment:6

Okay, cool !

I just noticed that the list of arguments is also available in GenericGraph.plot. That's an ugly copy/paste, that's for sure... I'm hesitant to remove it and link toward the auto-generated doc of the graph_plot module, however: what do you think?

Nathann

@nathanncohen
Copy link
Mannequin

nathanncohen mannequin commented Feb 11, 2016

comment:7

(adding Anne in case she cares: she mentionned the options of Graph.plot some time ago)

@jm58660
Copy link
Mannequin Author

jm58660 mannequin commented Feb 11, 2016

comment:8

Replying to @nathanncohen:

I just noticed that the list of arguments is also available in GenericGraph.plot. That's an ugly copy/paste, that's for sure... I'm hesitant to remove it and link toward the auto-generated doc of the graph_plot module, however: what do you think?

I think that #13827 is what we really need. This is phase zero, enabling features that are already there.

@anneschilling
Copy link

comment:9

What I recently wished existed was the following: internally the vertices are labelled by something. I wanted to be able to give a function f from the internal labels to some other labels (for printing). The printing labels would not necessarily need to distinct for all vertices (i.e. it could be a non-injective map). If x is an internal label, the plot would print f(x) next to the label.

Would this be easy to add?

@nathanncohen
Copy link
Mannequin

nathanncohen mannequin commented Feb 11, 2016

comment:10

Would this be easy to add?

Yes it is. This kind of code would be very similar to the other options already available, so you need not fear to hit a block if you give it a try.

@jm58660
Copy link
Mannequin Author

jm58660 mannequin commented Feb 11, 2016

comment:11

Ticket #15206 is about non-injective relabeling. Please add your code under it.

@jm58660
Copy link
Mannequin Author

jm58660 mannequin commented Jul 5, 2016

comment:12

Frédéric, got some minutes? This one is should be trivial.

@paulmasson
Copy link
Mannequin

paulmasson mannequin commented Jul 10, 2016

Reviewer: Paul Masson

@paulmasson
Copy link
Mannequin

paulmasson mannequin commented Jul 10, 2016

comment:13

Eight doctests failed: please fix them.

Also, in the documentation "tickness" needs to be corrected to "thickness".

@paulmasson paulmasson mannequin modified the milestones: sage-7.1, sage-7.3 Jul 10, 2016
@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 12, 2016

Changed commit from 55ab51a to 674361a

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 12, 2016

Branch pushed to git repo; I updated commit sha1. New commits:

674361aAdd thickness to list of options.

@jm58660
Copy link
Mannequin Author

jm58660 mannequin commented Jul 12, 2016

comment:16

OK, now the errors seem to be gone.

@paulmasson
Copy link
Mannequin

paulmasson mannequin commented Jul 12, 2016

comment:17

Doctests pass. Documentation builds. Example runs as expected.

@vbraun
Copy link
Member

vbraun commented Jul 13, 2016

Changed branch from u/jmantysalo/graph_plotting_thickness to 674361a

@slel
Copy link
Member

slel commented Apr 20, 2018

comment:19

See also #6546, #21540.

@slel
Copy link
Member

slel commented Apr 20, 2018

Changed commit from 674361a to none

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants