Opened 8 years ago

Closed 7 years ago

#12982 closed enhancement (fixed)

McGeeGraph, Tutte12Cage, TutteCoxeterGraph, WagnerGraph

Reported by: ncohen Owned by: jason, ncohen, rlm
Priority: major Milestone: sage-5.1
Component: graph theory Keywords: sd40.5
Cc: wdj, kini, dimpase Merged in: sage-5.1.beta5
Authors: Nathann Cohen Reviewers: Keshav Kini
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: #12942, #12945, #12952, #12971, #12980, #12981 Stopgaps:

Description (last modified by ncohen)

Adds some other graphs. I did not find any interesting embedding for Tutte's 12-cage, and I really tried :-/

Nathann

Apply:

Attachments (3)

trac_12982.patch (10.4 KB) - added by ncohen 8 years ago.
trac_12982.reviewer.patch (5.8 KB) - added by kini 8 years ago.
apply to $SAGE_ROOT/devel/sage
trac_12982.rebased.patch (10.4 KB) - added by kini 8 years ago.
apply to $SAGE_ROOT/devel/sage

Download all attachments as: .zip

Change History (9)

Changed 8 years ago by ncohen

comment:1 Changed 8 years ago by ncohen

  • Status changed from new to needs_review

comment:2 Changed 8 years ago by ncohen

  • Dependencies set to #12942, #12945, #12952, #12971, #12980, #12981

Changed 8 years ago by kini

apply to $SAGE_ROOT/devel/sage

Changed 8 years ago by kini

apply to $SAGE_ROOT/devel/sage

comment:3 Changed 8 years ago by kini

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

Another review patch for you! This time your patch needed to be rebased on my earlier review patches too...

Other than reformatting, I changed some doctests which seem to be copied and pasted from earlier graphs you made - they are retesting the default value of embedding, when they should be testing the non-default one. I also reused a previously generated graph instead of regenerating it, in line 4642 of graph_generators.py.

If you agree with the review patch, we can set the ticket to positive review.

patchbot: apply trac_12982.rebased.patch trac_12982.reviewer.patch

comment:4 Changed 8 years ago by ncohen

  • Description modified (diff)
  • Status changed from needs_review to positive_review

All tests pass, and the modifications you made are good ! Thank you again ! :-)

Nathann

comment:5 Changed 8 years ago by kini

Sure, thanks for your work :)

comment:6 Changed 7 years ago by jdemeyer

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