Opened 9 years ago

Closed 9 years ago

Last modified 9 years ago

#15044 closed enhancement (fixed)

Meredith Graph constructor

Reported by: Nathann Cohen Owned by:
Priority: major Milestone: sage-6.1
Component: graph theory Keywords:
Cc: Merged in:
Authors: Nathann Cohen Reviewers: Rob Beezer, Volker Braun
Report Upstream: N/A Work issues:
Branch: u/ncohen/15044 (Commits, GitHub, GitLab) Commit: f1bf7bb65b605388ab5b1b4e48bb48011ebf0a41
Dependencies: Stopgaps:

Status badges

Description

http://en.wikipedia.org/wiki/Meredith_graph

Here it is. As my first Git patch :-P

Nathann

Change History (23)

comment:1 Changed 9 years ago by Nathann Cohen

Status: newneeds_review

comment:2 Changed 9 years ago by Rob Beezer

Alright, Nathann. Two can play this game. My first attempt to review a patch with git. ;-) Went very well, thank-you.

HTML documentation. Meredith Graph as link in 3xN table is not a link. I see it as an input to __append_to_doc. I have tried make doc and various other incantations. Do I need to do something special? This is all I can see that would hold up a positive review.

Very neat graph. Plot is pretty. Factored characteristic polynomial in Sage matches Wikipedia's so I'd say the probability is very high that the graph is correct. ;-)

Rob

comment:3 Changed 9 years ago by Nathann Cohen

Hellooooooooooo Rob !

Nicely spotted. I had written "Mederith" instead of "Meredith", and of course it had to fail somewhere :-P

I just updated my branch by changing the last commit instead of commiting a one-line change, which I am told is a dangerous thing to do in general. Doing dangerous things is the best way to learn quickly how it all works :-P

I hope that it will be fine on your side ! :-)

Have fuuuuuuuuuuuuuun !

Nathann

comment:4 in reply to:  3 ; Changed 9 years ago by Rob Beezer

Commit: 90b4aa8eb215dd3567fefff2d09576f555c5c370
Reviewers: Rob Beezer
Status: needs_reviewpositive_review

Replying to ncohen:

Nicely spotted. I had written "Mederith" instead of "Meredith", and of course it had to fail somewhere :-P

I kept misspelling it while I searched the source, etc, so I should have figured that out! ;-)

I hope that it will be fine on your side ! :-)

All fine on your side. I spent *forever* trying to make it a "tracking" branch on my side. Maybe details on sage-git later today.

All good for a positive review.

I added a commit hash in the "Commit" field - I hope that is right.

Rob

comment:5 in reply to:  4 Changed 9 years ago by Nathann Cohen

Helloooo Rob !!!

All fine on your side. I spent *forever* trying to make it a "tracking" branch on my side. Maybe details on sage-git later today.

Hmmmm... I don't even know what that means yet :-P

All good for a positive review.

Thanks !!

I added a commit hash in the "Commit" field - I hope that is right.

Hmmmmm.. Some time ago I think I saw a discussion on sage-git that I didn't understand at that time about whether we should use branches or commits references. And I think that we should use branches instead, for the following reason :

  • This Meredith patch was uploaded yesterday, when the git version of Sage was still version 5.11.rc0
  • I then updated my version of sage-git to 5.12 with a git pull
  • I now want to write a patch #15049 that is based upon this patch

The point now, is that this patch's commit was created before 5.12. Hence if I want to write #15049 atop this patch, I can't be above 5.12 too. What I could do, however, is rebase this patch atop 5.12, hence updating the u/ncohen/meredith branch, then write #15049 on this new meredith branch which is above 5.12.

The problem is that the hash of a commit changes when you rebase it. Or at least I found no way to avoid that O_o

Sooooooo hoping that it is not a problem, I will try to rebase this patch above 5.12 (and there should not be any conflict), update the commit message to match the new version of the meredith commit, then base #15049 atop of that.

I hope all of that was clear :-P

Have fuuuuuuuuuun ! And thank you again :-)

Nathann

Version 0, edited 9 years ago by Nathann Cohen (next)

comment:6 Changed 9 years ago by Nathann Cohen

Commit: 90b4aa8eb215dd3567fefff2d09576f555c5c3702023ac74ff65df6a92f962c70c4d058a0722e6c9

comment:7 Changed 9 years ago by Jeroen Demeyer

Authors: Nathann Cohen
Milestone: sage-5.12sage-6.0

comment:8 Changed 9 years ago by Jeroen Demeyer

Status: positive_reviewneeds_work

Did you mean # long time instead of # long?

comment:9 in reply to:  8 Changed 9 years ago by Nathann Cohen

Commit: 2023ac74ff65df6a92f962c70c4d058a0722e6c9

Did you mean # long time instead of # long?

Right. I just replaced it with a "long time".

And because I had to make this small new commit, and because I merged it with this Meredith patch, I had to rebase and reupload #15049. Because git remembers the hash of its dependencies, and so even if the small modifications that you make do not create any conflict you have to rebase everything above T_T

This is going to be painful in the very near future O_O

Nathann

comment:10 Changed 9 years ago by Nathann Cohen

Status: needs_workpositive_review

comment:11 Changed 9 years ago by Nathann Cohen

Branch: u/ncohen/meredith_graphu/ncohen/15044

(just changing the branch's name)

Nathann

comment:12 Changed 9 years ago by git

Commit: cc9d0c3c814ffcd464765b0d24ff835062bef587
Status: positive_reviewneeds_review

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:

[changeset:cc9d0c3]Merge branch 'start' into meredith
[changeset:ac53c3d]Meredith Graph constructor

comment:13 Changed 9 years ago by Nathann Cohen

I just rebased it... I hope it was effective :-/

Nathann

comment:14 Changed 9 years ago by Karl-Dieter Crisman

How do we review things NOT using git that are in git? I have some motivation to review #15054 (a friend who really loves snarks and needs to be encourage to use Sage to explore them) but am not going to have time to learn the new workflow yet (despite Volker's excellent slides from the Sage Days this week).

comment:15 Changed 9 years ago by git

Commit: cc9d0c3c814ffcd464765b0d24ff835062bef587d66b7d10231d055a4dda0b11c9ee3dfdb2a9f79f

Branch pushed to git repo; I updated commit sha1. Last 10 new commits:

[changeset:d66b7d1]Meredith Graph constructor
[changeset:3b15578]Merging Sage-5.12.beta5, newest dev scripts, and the doctest fixes.
[changeset:1456c52]Merge branch 'ticket/14482' into public/sage-git/master
[changeset:b890215]Merge branch 'ticket/14482' into public/sage-git/master
[changeset:d8713eb]Merge remote-tracking branch 'origin/build_system' into public/sage-git/master
[changeset:970090d]Merge branch 'u/ohanar/build_system'
[changeset:cf14c84]Merge branch 'ticket/14482' into public/sage-git/master
[changeset:060184c]Merge branch 'ticket/14482' into public/sage-git/master
[changeset:b95a820]Merge branch 'ticket/14482' into public/sage-git/master
[changeset:a10bc4f]Merge branch 'ticket/14482'

comment:16 Changed 9 years ago by Nathann Cohen

Just rebased it. Seems better this way :-P

Nathann

comment:17 Changed 9 years ago by Rob Beezer

Hellooooooooooo Nathann,

I have not forgotten this one. I am learning git on some smaller scale projects, and I'm close to trying this one again. I *will* be back, and better prepared.

Rob

comment:18 Changed 9 years ago by Nathann Cohen

Thaaaaaaanks :-D

Nathann

comment:19 Changed 9 years ago by git

Commit: d66b7d10231d055a4dda0b11c9ee3dfdb2a9f79ff1bf7bb65b605388ab5b1b4e48bb48011ebf0a41

Branch pushed to git repo; I updated commit sha1. Last 10 new commits:

[changeset:f1bf7bb]Rebase on 5.13.beta0
[changeset:d66b7d1]Meredith Graph constructor
[changeset:3b15578]Merging Sage-5.12.beta5, newest dev scripts, and the doctest fixes.
[changeset:1456c52]Merge branch 'ticket/14482' into public/sage-git/master
[changeset:b890215]Merge branch 'ticket/14482' into public/sage-git/master
[changeset:d8713eb]Merge remote-tracking branch 'origin/build_system' into public/sage-git/master
[changeset:970090d]Merge branch 'u/ohanar/build_system'
[changeset:cf14c84]Merge branch 'ticket/14482' into public/sage-git/master
[changeset:060184c]Merge branch 'ticket/14482' into public/sage-git/master
[changeset:b95a820]Merge branch 'ticket/14482' into public/sage-git/master

comment:20 Changed 9 years ago by For batch modifications

Milestone: sage-6.0sage-6.1

comment:21 Changed 9 years ago by Volker Braun

Reviewers: Rob BeezerRob Beezer, Volker Braun
Status: needs_reviewpositive_review

lgtm

comment:22 Changed 9 years ago by Volker Braun

Resolution: fixed
Status: positive_reviewclosed

comment:23 Changed 9 years ago by Nathann Cohen

Thaaaaaaaaaaaanks !! ;-)

Nathann

Note: See TracTickets for help on using tickets.