Opened 7 years ago
Closed 7 years ago
#16215 closed enhancement (fixed)
Error raised when nonmulti(di)graph receive multiple edges as input
Reported by:  ncohen  Owned by:  

Priority:  major  Milestone:  sage6.2 
Component:  graph theory  Keywords:  graph, digraph, multiedges, error 
Cc:  Merged in:  
Authors:  Vincent Delecroix  Reviewers:  Nathann Cohen 
Report Upstream:  N/A  Work issues:  
Branch:  f6102dd (Commits)  Commit:  f6102dd6b2fdb2f47c08e093db135a85243a156b 
Dependencies:  Stopgaps: 
Description (last modified by )
sage: Graph([(0,1),(0,1)],multiedges=False) Traceback (most recent call last): ... ValueError: Nonmultigraph 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
Change History (12)
comment:1 Changed 7 years ago by
 Status changed from new to needs_review
comment:2 Changed 7 years ago by
 Branch set to u/ncohen/16215
 Commit set to 7f50072dc0ab7cd38e5a2af14534b4ce012fa9f2
comment:3 Changed 7 years ago by
 Description modified (diff)
 Reviewers set to Vincent Delecroix
 Status changed from needs_review to needs_work
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.
comment:4 Changed 7 years ago by
But it's funny... And it has been there from the start, and there is the same in Graph !
Nathann
comment:5 Changed 7 years ago by
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
comment:6 Changed 7 years ago by
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
comment:7 Changed 7 years ago by
 Branch changed from u/ncohen/16215 to u/vdelecroix/16215
 Commit changed from 7f50072dc0ab7cd38e5a2af14534b4ce012fa9f2 to f6102dd6b2fdb2f47c08e093db135a85243a156b
 Keywords graph digraph multiedges error added
 Reviewers Vincent Delecroix deleted
 Status changed from needs_work to needs_review
 Summary changed from A missing "import" in DiGraph to Error raised when nonmulti(di)graph receive multiple edges as input
We now have
sage: Graph([(0,1),(0,1)], multiedges=False) Traceback (most recent call last): ... ValueError: Nonmultigraph got several edges (0,1) sage: DiGraph([(0,1),(0,1)], multiedges=False) Traceback (most recent call last): ... ValueError: Nonmultidigraph got several edges (0,1)
Needs review.
New commits:
f6102dd  Error when multiple edges are sent to a Nonmulti(di)graph

comment:8 Changed 7 years ago by
 Status changed from needs_review to positive_review
All tests pass, good to go !
Nathann
comment:9 Changed 7 years ago by
Thanks !
comment:10 followup: ↓ 11 Changed 7 years ago by
Reviewer name
comment:11 in reply to: ↑ 10 Changed 7 years ago by
 Reviewers set to Nathann Cohen
comment:12 Changed 7 years ago by
 Branch changed from u/vdelecroix/16215 to f6102dd6b2fdb2f47c08e093db135a85243a156b
 Resolution set to fixed
 Status changed from positive_review to closed
New commits:
trac #16215: A missing import