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)

trac_12989.patch (6.5 KB) - added by ncohen 7 years ago.
trac_12989.reviewer.patch (7.9 KB) - added by ncohen 7 years ago.
apply to $SAGE_ROOT/devel/sage

Download all attachments as: .zip

Change History (14)

comment:1 Changed 7 years ago by ncohen

  • Status changed from new to needs_review

comment:2 follow-up: Changed 7 years ago by kini

  • Keywords sd40.5 added
  • Reviewers set to Keshav Kini

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

comment:3 Changed 7 years ago by kini

Oops.

patchbot: apply trac_12989.patch trac_12989.reviewer.patch

comment:4 in reply to: ↑ 2 Changed 7 years ago by ncohen

  • 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 jdemeyer

  • 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 ncohen

comment:6 Changed 7 years ago by ncohen

  • 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 jdemeyer

  • Milestone changed from sage-5.1 to sage-5.2

comment:8 Changed 7 years ago by jdemeyer

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 jdemeyer

  • Status changed from positive_review to needs_work

Changed 7 years ago by ncohen

apply to $SAGE_ROOT/devel/sage

comment:10 Changed 7 years ago by ncohen

  • Status changed from needs_work to needs_review

Patch updated !

Nathann

comment:11 Changed 7 years ago by ncohen

  • Status changed from needs_review to positive_review

comment:12 Changed 7 years ago by jdemeyer

  • Merged in set to sage-5.2.beta0
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.