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

edge_incident bug in generic_graph.py #9581

Closed
videlec opened this issue Jul 23, 2010 · 15 comments
Closed

edge_incident bug in generic_graph.py #9581

videlec opened this issue Jul 23, 2010 · 15 comments

Comments

@videlec
Copy link
Contributor

videlec commented Jul 23, 2010

Currently, the edge_incident method of generic graph calls edge_boundary which first take a lot of time and secondly does not work

sage: G = Graph(loops=True)
sage: G.add_edge(0,0)
sage: G.edges()
[(0, 0, None)]
sage: list(G.edge_iterator(0))
[(0, 0, None)]
sage: G.edges_incident(0)
[]

The ticket also aims to reduce multiple calls (edge_boundary does not call directly edge_iterator as it should).

It is also the occasion to add some doc and correct some typos.

Apply first :

CC: @nathanncohen

Component: graph theory

Author: Vincent Delecroix

Reviewer: Nathann Cohen

Merged: sage-4.6.1.alpha0

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

@videlec videlec added this to the sage-4.6 milestone Jul 23, 2010
@videlec videlec self-assigned this Jul 23, 2010
@videlec

This comment has been minimized.

@nathanncohen
Copy link
Mannequin

nathanncohen mannequin commented Jul 23, 2010

comment:2

Excellent ! Thank youuuuuuuuuuuuuu !!

Your patch is very nice, applies fine and everything.. I would just like to append a short line, because of a missing "if". If you agree with this, let's say this ticket is positively reviewed ! :-)

Nathann

@nathanncohen nathanncohen mannequin added the s: needs review label Jul 23, 2010
@nathanncohen
Copy link
Mannequin

nathanncohen mannequin commented Jul 23, 2010

Attachment: trac_9581-fix.patch.gz

@videlec
Copy link
Contributor Author

videlec commented Jul 23, 2010

comment:3

Nathan, Why did you put this ticket as needs_review? It seems to be important to be a lot more explicit in the definition of each function of generic_graph and implement all the cases in examples... perhaps it is the matter of another ticket...

@nathanncohen
Copy link
Mannequin

nathanncohen mannequin commented Jul 24, 2010

comment:4

Hello ! Well, if you think it needs more documenation or tests, this ticket certainly is the one that should contain it... I thought the behaviour of these functions did not change that much, only "internal modifications", so... But I'm sorry for this, all you just said is better done here ! :-)

@videlec
Copy link
Contributor Author

videlec commented Oct 10, 2010

apply only this patch which takes care of Nathan remark

@videlec
Copy link
Contributor Author

videlec commented Oct 10, 2010

comment:5

Attachment: trac_9581-edge_incident.patch.gz

@nathanncohen
Copy link
Mannequin

nathanncohen mannequin commented Oct 10, 2010

comment:6

Hello !!! I can not apply this patch on 4.6.alpha3, looks like it needs to be rebased ^^;

Nathann

@videlec
Copy link
Contributor Author

videlec commented Oct 11, 2010

rebased version (apply only this one)

@videlec
Copy link
Contributor Author

videlec commented Oct 11, 2010

comment:7

Attachment: trac_9581-edge_incident.2.patch.gz

@nathanncohen
Copy link
Mannequin

nathanncohen mannequin commented Oct 11, 2010

comment:8

Positive review to this rebased version :-)

Nathann

@nathanncohen

This comment has been minimized.

@jdemeyer jdemeyer removed this from the sage-4.6 milestone Oct 23, 2010
@jdemeyer jdemeyer added this to the sage-4.6.1 milestone Oct 23, 2010
@jdemeyer
Copy link

Reviewer: Nathann Cohen

@jdemeyer
Copy link

Changed author from vdelecroix to Vincent Delecroix

@jdemeyer
Copy link

jdemeyer commented Nov 1, 2010

Merged: sage-4.6.1.alpha0

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

2 participants