Opened 5 years ago
Closed 5 years ago
#10781 closed enhancement (fixed)
add Shrikhande graph to the common graphs database
Reported by: | kini | Owned by: | jason, ncohen, rlm |
---|---|---|---|
Priority: | major | Milestone: | sage-4.7 |
Component: | graph theory | Keywords: | |
Cc: | Merged in: | sage-4.7.alpha3 | |
Authors: | Keshav Kini | Reviewers: | Nathann Cohen, Jeroen Demeyer |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description (last modified by )
Part of the larger project at #9136 .
Apply trac_10781-Shrikhande-graph.patch
Attachments (2)
Change History (14)
comment:1 Changed 5 years ago by
- Status changed from new to needs_review
comment:2 Changed 5 years ago by
- Owner changed from kini to jason, ncohen, rlm
comment:3 Changed 5 years ago by
- Milestone set to sage-4.7
- Reviewers set to Nathann Cohen
Nice addition ! Passes all tests and does its work, except for one small detail : we try to keep the lines below 80 characters in Sage's code. I uploaded a reviewer's patch, and if you agree with it this ticket can be set to "positive review"
Nathann
Changed 5 years ago by
comment:4 follow-up: ↓ 5 Changed 5 years ago by
Of course, that is fine with me. I'll keep the 80-character width limit in mind next time.
If you like, I can merge your patch into mine and reupload it. Shall I?
comment:5 in reply to: ↑ 4 Changed 5 years ago by
If you like, I can merge your patch into mine and reupload it. Shall I?
Oh. I did it myself just to avoid you the trouble of doing this. Anyway you're the only Author of this patch, it's just to make the review quicker :-)
So it's up to you : just don't forget to set the ticket to "positive review" so that the release manager will eventually merge the two tickets !
Nathann
comment:6 follow-up: ↓ 7 Changed 5 years ago by
Ping ? :-)
Nathann
comment:7 in reply to: ↑ 6 Changed 5 years ago by
- Description modified (diff)
- Status changed from needs_review to positive_review
Replying to ncohen:
Ping ?
:-)
Nathann
Hey, sorry for the delay, I was traveling (on my way, indirectly, to Sage Days 29 !). I'm setting this ticket to "positive review" as you asked, but if you change your mind feel free to change it back :)
comment:8 Changed 5 years ago by
- Status changed from positive_review to needs_work
Since the hyperlink to the MathWorld and Wikipedia articles are not meant to be refered to from a different place, you should make links with two trailing underscores. See http://docutils.sourceforge.net/docs/ref/rst/restructuredtext.html#embedded-uris
Changed 5 years ago by
comment:9 follow-up: ↓ 10 Changed 5 years ago by
- Status changed from needs_work to needs_review
Hi Jeroen, thanks for your advice as usual! After looking through the link you provided, and also the file I am modifying ( sage/graphs/graph_generators.py ), it seems that all the other links to Wikipedia or MathWorld? are using single underscores. The only difference seemed to be that in my links, the text of the link (i.e. the name given to the reference) was attached to the context of the sentence it was in, rather than being a standalone phrase. I attempted to fix that, instead of doubling the underscores. Is that acceptable?
comment:10 in reply to: ↑ 9 Changed 5 years ago by
Replying to kini:
Hi Jeroen, thanks for your advice as usual! After looking through the link you provided, and also the file I am modifying ( sage/graphs/graph_generators.py ), it seems that all the other links to Wikipedia or MathWorld? are using single underscores. The only difference seemed to be that in my links, the text of the link (i.e. the name given to the reference) was attached to the context of the sentence it was in, rather than being a standalone phrase. I attempted to fix that, instead of doubling the underscores. Is that acceptable?
I guess this is acceptable, yes. I still have to build the documentation and see what it gives, but I guess it's okay.
comment:11 Changed 5 years ago by
- Reviewers changed from Nathann Cohen to Nathann Cohen, Jeroen Demeyer
- Status changed from needs_review to positive_review
comment:12 Changed 5 years ago by
- Merged in set to sage-4.7.alpha3
- Resolution set to fixed
- Status changed from positive_review to closed
Patch added. I'm assigning this ticket to jason, ncohen, and rlm because they are in charge of the blanket ticket #9136 .