Opened 10 years ago
Closed 10 years ago
#6774 closed enhancement (wontfix)
tour Graph Theory
Reported by: | ncohen | Owned by: | tba |
---|---|---|---|
Priority: | major | Milestone: | sage-duplicate/invalid/wontfix |
Component: | documentation | Keywords: | |
Cc: | Merged in: | ||
Authors: | Nathann Cohen | Reviewers: | Jason Grout |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description (last modified by )
Hello !
I thought it was a pity Sage contained nothing yet about Graph theory. This is what I have written to fix it... Merge any of your ideas in it, it can not hope to be complete !
And.... If I forgot your country when coloring the map of Western Europe, or if you do not stand to watch Western Europe being colored while your country is not, just add the data to the document ;-)
As mentionned, this ticket depends on:
Nathann
Attachments (2)
Change History (19)
Changed 10 years ago by
Changed 10 years ago by
comment:1 Changed 10 years ago by
I added stylistic/punctuation corrections. Positive review pending the examples actually working (I believe they need ncohen's patches to be merged in).
comment:2 Changed 10 years ago by
- Summary changed from [with patch, needs review] tour Graph Theory to [with patch, positive review] tour Graph Theory
I guess I should change this to "positive review", and just say it depends on whatever tickets implement the functionality in the examples.
comment:3 Changed 10 years ago by
- Merged in set to Sage 4.1.2.alpha2
- Resolution set to fixed
- Reviewers set to Jason Grout
- Status changed from new to closed
Merged patches in this order:
graph_tour.patch
trac-6774-graph-tour-review.patch
See #6952 for a follow-up to this ticket.
comment:4 follow-up: ↓ 5 Changed 10 years ago by
I assume that you merged the tickets that define the functions listed here (i.e., you were able to run doctests on this file and everything was okay). The functions in this file did not work for me with 4.1.1.alpha1, presumably because they hadn't been merged at that point.
comment:5 in reply to: ↑ 4 ; follow-up: ↓ 6 Changed 10 years ago by
Replying to jason:
I assume that you merged the tickets that define the functions listed here
Yes, I have done so.
(i.e., you were able to run doctests on this file and everything was okay).
Yes, with the two patches on this ticket all doctests in the tutorial pass.
comment:6 in reply to: ↑ 5 Changed 10 years ago by
Replying to mvngu:
Replying to jason:
I assume that you merged the tickets that define the functions listed here
Yes, I have done so.
No, you haven't. These functions are defined in #6679 and #6680, which haven't been reviewed, let alone merged. This ticket should have been marked as "depends on #6679 and #6680": we can't have functions mentioned in the tutorial which are not yet part of Sage. For now, the graph theory tour has been removed from the tutorial -- see #7149.
When the tickets #6679 and #6680 have been merged, then open up a new ticket to reinstate the graph theory tour (the corrected version, as of #6952). Before running doctests, apply the script patches from #6572 to make sure that everything is getting tested -- the current doctesting system is broken, so many doctests in .rst files are inadvertently skipped.
comment:7 Changed 10 years ago by
I would have prefered the fixing to be "let's review these two patches now to have a good Graph Theory Tour", but removing it for the moment is a good solution too.. Perhaps Minh just meant that he had applied the corresponding patches to test the tutorial..
The thing is that patches #6679 and #6680 define for Sage some of the most important functions in Graph Theory ( Flows, matching, graph coloring, connectivity, etc .. )
The first version is two month old, and since then I have had to update them each time class MIP is modified. I do not even know myself how many things I have yet to write based on these functions.. What could we do about it ? If the code is not commented enough, if the only things you need are references, tell me and they'll be there in a few seconds :-)
comment:8 Changed 10 years ago by
- Description modified (diff)
- Report Upstream set to N/A
- Resolution fixed deleted
- Status changed from closed to new
comment:9 Changed 10 years ago by
- Status changed from new to needs_review
comment:10 Changed 10 years ago by
- Status changed from needs_review to positive_review
comment:11 follow-up: ↓ 12 Changed 10 years ago by
I thought that we didn't want things in the tutorial that depended on optional packages.
comment:12 in reply to: ↑ 11 Changed 10 years ago by
Replying to mhansen:
I thought that we didn't want things in the tutorial that depended on optional packages.
That is correct. Anything in the attached patches that uses optional packages should be removed. A new patch should be attached that still gives a tour of graph theory in Sage, but without using any optional packages.
comment:13 follow-up: ↓ 15 Changed 10 years ago by
Is it just because it would be a legal problem for Sage to claim as "available" functions that we cannot provide without the addition of a software we are not allowed to embed ?
In this case I understand this decision, but you have to consider the fact that it is very likely 99% of Sage's graph theoretical functions will depend on Linear Programming by the time people REWRITE the functions I defined using Linear Programming through other means.
I'll just try again to find a free LP solver or an old version of GLPK. And in the meantine I'll just update my personnal tutorial on Graphs (available pretty soon).. :-)
comment:14 Changed 10 years ago by
Besides, this can be updated because of ticket #6813 which just got merged... We have much more that eastern europe in Sage now :-)
comment:15 in reply to: ↑ 13 Changed 10 years ago by
Replying to ncohen:
Is it just because it would be a legal problem for Sage to claim as "available" functions that we cannot provide without the addition of a software we are not allowed to embed ?
It's because everything in the tutorial should "just work". You shouldn't have to install anything extra, it should work right out of the box.
comment:16 Changed 10 years ago by
Can we close this as "won't fix"?
comment:17 Changed 10 years ago by
- Merged in Sage 4.1.2.alpha2 deleted
- Milestone changed from sage-4.1.2 to sage-duplicate/invalid/wontfix
- Resolution set to wontfix
- Status changed from positive_review to closed
- Summary changed from [with patch, positive review] tour Graph Theory to tour Graph Theory
The graph theory tour as is cannot be added to the tutorial since it depends on an optional package. However, you can open a ticket to add that tour to the Constructions document.
apply on top of previous patch