Opened 7 years ago
Closed 7 years ago
#12989 closed enhancement (fixed)
Ellingham–Horton graphs
Reported by: | ncohen | Owned by: | jason, ncohen, rlm |
---|---|---|---|
Priority: | major | Milestone: | sage-5.2 |
Component: | graph theory | Keywords: | sd40.5 |
Cc: | wdj, kini, dimpase | Merged in: | sage-5.2.beta0 |
Authors: | Nathann Cohen | Reviewers: | Keshav Kini |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | #12942, #12945, #12952, #12971, #12980, #12981, #12982 | Stopgaps: |
Description
This patch adds the two Ellingham-Horton graphs to Sage's constructors.
http://en.wikipedia.org/wiki/Ellingham%E2%80%93Horton_graph
Ugly things. I am not eager to add other "topological graphs" again :-P
Nathann
Attachments (2)
Change History (14)
comment:1 Changed 7 years ago by
- Status changed from new to needs_review
comment:2 follow-up: ↓ 4 Changed 7 years ago by
- Keywords sd40.5 added
- Reviewers set to Keshav Kini
comment:3 Changed 7 years ago by
Oops.
patchbot: apply trac_12989.patch trac_12989.reviewer.patch
comment:4 in reply to: ↑ 2 Changed 7 years ago by
- Status changed from needs_review to positive_review
Other than formatting, I fixed the rST wikipedia links, standardized the graph names, and eliminated the needless variable
x
which you were using during development I guess.
Thaaaaaaaaaaaaaaaanks !! :-)
Nathann
comment:5 Changed 7 years ago by
- Status changed from positive_review to needs_work
This takes several minutes to doctest, which is too much.
More generally: you cannot keep adding stuff to graph_generators.py
forever. At some point, it will be needed to split up this file (perhaps into a new directory sage/graphs/generators
).
Changed 7 years ago by
comment:6 Changed 7 years ago by
- Status changed from needs_work to positive_review
Updated !!!
As for the length of this file I agree that I will have to split it, but could we merge the tickets anyway in the meantime ? They are a veeeeeeeeery long of interdependent tickets and it would really be a mess to rebase them all ^^;
Nathann
comment:7 Changed 7 years ago by
- Milestone changed from sage-5.1 to sage-5.2
comment:8 Changed 7 years ago by
The reviewer patch doesn't apply cleanly:
applying /release/merger/patches/trac_12989.reviewer.patch patching file sage/graphs/graph_generators.py Hunk #2 FAILED at 3417 Hunk #3 FAILED at 3506 2 out of 3 hunks FAILED -- saving rejects to file sage/graphs/graph_generators.py.rej abort: patch failed to apply
comment:9 Changed 7 years ago by
- Status changed from positive_review to needs_work
comment:10 Changed 7 years ago by
- Status changed from needs_work to needs_review
Patch updated !
Nathann
comment:11 Changed 7 years ago by
- Status changed from needs_review to positive_review
comment:12 Changed 7 years ago by
- Merged in set to sage-5.2.beta0
- Resolution set to fixed
- Status changed from positive_review to closed
And one last review patch! (For now :) )
Other than formatting, I fixed the rST wikipedia links, standardized the graph names, and eliminated the needless variable
x
which you were using during development I guess.If you agree with the review patch, we can set this ticket to positive review.
patchbot: apply trac-12989.patch trac_12989.reviewer.patch