Opened 7 years ago
Closed 7 years ago
#16215 closed enhancement (fixed)
Error raised when non-multi(di)graph receive multiple edges as input
Reported by: | ncohen | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-6.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: 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
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 non-multi(di)graph receive multiple edges as input
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:
f6102dd | Error when multiple edges are sent to a Non-multi(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 follow-up: ↓ 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