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

Error raised when non-multi(di)graph receive multiple edges as input #16215

Closed
nathanncohen mannequin opened this issue Apr 23, 2014 · 21 comments
Closed

Error raised when non-multi(di)graph receive multiple edges as input #16215

nathanncohen mannequin opened this issue Apr 23, 2014 · 21 comments

Comments

@nathanncohen
Copy link
Mannequin

nathanncohen mannequin commented Apr 23, 2014

sage: Graph([(0,1),(0,1)],multiedges=False)
Traceback (most recent call last):
...
ValueError: Non-multigraph input dict has multiple edges (0,1)
sage: DiGraph([(0,1),(0,1)],multiedges=False)
Traceback (most recent call last):
...
NameError: global name 'choice' is not defined

The first message is much clearer :-P

Nathann

Component: graph theory

Keywords: digraph, multiedges, error

Author: Vincent Delecroix

Branch/Commit: f6102dd

Reviewer: Nathann Cohen

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

@nathanncohen nathanncohen mannequin added this to the sage-6.2 milestone Apr 23, 2014
@nathanncohen
Copy link
Mannequin Author

nathanncohen mannequin commented Apr 23, 2014

New commits:

7f50072trac #16215: A missing import

@nathanncohen
Copy link
Mannequin Author

nathanncohen mannequin commented Apr 23, 2014

Branch: u/ncohen/16215

@nathanncohen
Copy link
Mannequin Author

nathanncohen mannequin commented Apr 23, 2014

Commit: 7f50072

@videlec
Copy link
Contributor

videlec commented Apr 23, 2014

comment:3

Could you instead replace

choice([v for v in data[u] if data[u].count(v) > 1])

with

(v for v in data[u] if data[u].count(v) > 1).next()

or anything similar. The function choice involves some randomness that is useless here.

@videlec
Copy link
Contributor

videlec commented Apr 23, 2014

Reviewer: Vincent Delecroix

@videlec

This comment has been minimized.

@nathanncohen
Copy link
Mannequin Author

nathanncohen mannequin commented Apr 23, 2014

comment:4

But it's funny... And it has been there from the start, and there is the same in Graph !

Nathann

@videlec
Copy link
Contributor

videlec commented Apr 23, 2014

comment:5

It is funny, but it was wrong and nobody noticed... so the simplest, the best. Especially in the code of a complicated constructor (~600 lines for the one in Graph). And as you mentioned, It would be better to remove the one in Graph as well ;-P

Vincent

@nathanncohen
Copy link
Mannequin Author

nathanncohen mannequin commented Apr 23, 2014

comment:6

For me the simplest is to add the missing line. If you want to solve it differently, add a commit. There is nothing tricky involved, you do not need me to write it.

Nathann

@videlec
Copy link
Contributor

videlec commented Apr 23, 2014

Changed branch from u/ncohen/16215 to u/vdelecroix/16215

@videlec
Copy link
Contributor

videlec commented Apr 23, 2014

Changed commit from 7f50072 to f6102dd

@videlec
Copy link
Contributor

videlec commented Apr 23, 2014

comment:7

We now have

sage: Graph([(0,1),(0,1)], multiedges=False)
Traceback (most recent call last):
...
ValueError: Non-multigraph got several edges (0,1)
sage: DiGraph([(0,1),(0,1)], multiedges=False)
Traceback (most recent call last):
...
ValueError: Non-multidigraph got several edges (0,1)

Needs review.


New commits:

f6102ddError when multiple edges are sent to a Non-multi(di)graph

@videlec
Copy link
Contributor

videlec commented Apr 23, 2014

Changed reviewer from Vincent Delecroix to none

@videlec
Copy link
Contributor

videlec commented Apr 23, 2014

Changed keywords from none to digraph, multiedges, error

@videlec
Copy link
Contributor

videlec commented Apr 23, 2014

Changed author from Nathann Cohen to Vincent Delecroix

@videlec videlec changed the title A missing "import" in DiGraph Error raised when non-multi(di)graph receive multiple edges as input Apr 23, 2014
@nathanncohen
Copy link
Mannequin Author

nathanncohen mannequin commented Apr 23, 2014

comment:8

All tests pass, good to go !

Nathann

@videlec
Copy link
Contributor

videlec commented Apr 23, 2014

comment:9

Thanks !

@vbraun
Copy link
Member

vbraun commented Apr 28, 2014

comment:10

Reviewer name

@videlec
Copy link
Contributor

videlec commented Apr 28, 2014

Reviewer: Nathann Cohen

@videlec
Copy link
Contributor

videlec commented Apr 28, 2014

comment:11

Replying to @vbraun:

Reviewer name

Sorry. Here it is.

@vbraun
Copy link
Member

vbraun commented Apr 30, 2014

Changed branch from u/vdelecroix/16215 to f6102dd

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