## #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: |

### Description

### Change History (23)

### comment:1 Changed 9 years ago by

Status: | new → needs_review |
---|

### comment:2 Changed 9 years ago by

### comment:3 follow-up: 4 Changed 9 years ago by

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 follow-up: 5 Changed 9 years ago by

Commit: | → 90b4aa8eb215dd3567fefff2d09576f555c5c370 |
---|---|

Reviewers: | → Rob Beezer |

Status: | needs_review → positive_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 Changed 9 years ago by

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

### comment:6 Changed 9 years ago by

Commit: | 90b4aa8eb215dd3567fefff2d09576f555c5c370 → 2023ac74ff65df6a92f962c70c4d058a0722e6c9 |
---|

### comment:7 Changed 9 years ago by

Authors: | → Nathann Cohen |
---|---|

Milestone: | sage-5.12 → sage-6.0 |

### comment:8 follow-up: 9 Changed 9 years ago by

Status: | positive_review → needs_work |
---|

Did you mean `# long time`

instead of `# long`

?

### comment:9 Changed 9 years ago by

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

Status: | needs_work → positive_review |
---|

### comment:11 Changed 9 years ago by

Branch: | u/ncohen/meredith_graph → u/ncohen/15044 |
---|

(just changing the branch's name)

Nathann

### comment:12 Changed 9 years ago by

Commit: | → cc9d0c3c814ffcd464765b0d24ff835062bef587 |
---|---|

Status: | positive_review → needs_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:14 Changed 9 years ago by

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

Commit: | cc9d0c3c814ffcd464765b0d24ff835062bef587 → d66b7d10231d055a4dda0b11c9ee3dfdb2a9f79f |
---|

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:17 Changed 9 years ago by

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:19 Changed 9 years ago by

Commit: | d66b7d10231d055a4dda0b11c9ee3dfdb2a9f79f → f1bf7bb65b605388ab5b1b4e48bb48011ebf0a41 |
---|

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

Milestone: | sage-6.0 → sage-6.1 |
---|

### comment:21 Changed 9 years ago by

Reviewers: | Rob Beezer → Rob Beezer, Volker Braun |
---|---|

Status: | needs_review → positive_review |

lgtm

### comment:22 Changed 9 years ago by

Resolution: | → fixed |
---|---|

Status: | positive_review → closed |

**Note:**See TracTickets for help on using tickets.

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