#15044 closed enhancement (fixed)
Meredith Graph constructor
Reported by: | ncohen | 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 8 years ago by
- Status changed from new to needs_review
comment:2 Changed 8 years ago by
comment:3 follow-up: ↓ 4 Changed 8 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 in reply to: ↑ 3 ; follow-up: ↓ 5 Changed 8 years ago by
- Commit set to 90b4aa8eb215dd3567fefff2d09576f555c5c370
- Reviewers set to Rob Beezer
- Status changed from needs_review to 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 in reply to: ↑ 4 Changed 8 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 8 years ago by
- Commit changed from 90b4aa8eb215dd3567fefff2d09576f555c5c370 to 2023ac74ff65df6a92f962c70c4d058a0722e6c9
comment:7 Changed 8 years ago by
- Milestone changed from sage-5.12 to sage-6.0
comment:8 follow-up: ↓ 9 Changed 8 years ago by
- Status changed from positive_review to needs_work
Did you mean # long time
instead of # long
?
comment:9 in reply to: ↑ 8 Changed 8 years ago by
- Commit 2023ac74ff65df6a92f962c70c4d058a0722e6c9 deleted
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 8 years ago by
- Status changed from needs_work to positive_review
comment:11 Changed 8 years ago by
- Branch changed from u/ncohen/meredith_graph to u/ncohen/15044
(just changing the branch's name)
Nathann
comment:12 Changed 8 years ago by
- Commit set to cc9d0c3c814ffcd464765b0d24ff835062bef587
- Status changed from positive_review to 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:13 Changed 8 years ago by
I just rebased it... I hope it was effective :-/
Nathann
comment:14 Changed 8 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 8 years ago by
- Commit changed from cc9d0c3c814ffcd464765b0d24ff835062bef587 to 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:16 Changed 8 years ago by
Just rebased it. Seems better this way :-P
Nathann
comment:17 Changed 8 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:18 Changed 8 years ago by
Thaaaaaaanks :-D
Nathann
comment:19 Changed 8 years ago by
- Commit changed from d66b7d10231d055a4dda0b11c9ee3dfdb2a9f79f to 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 8 years ago by
- Milestone changed from sage-6.0 to sage-6.1
comment:21 Changed 8 years ago by
- Reviewers changed from Rob Beezer to Rob Beezer, Volker Braun
- Status changed from needs_review to positive_review
lgtm
comment:22 Changed 8 years ago by
- Resolution set to fixed
- Status changed from positive_review to closed
comment:23 Changed 8 years ago by
Thaaaaaaaaaaaanks !! ;-)
Nathann
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 triedmake 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