Opened 9 years ago

Closed 9 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 ncohen)

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)

graph_tour.patch (10.0 KB) - added by ncohen 9 years ago.
trac-6774-graph-tour-review.patch (11.8 KB) - added by jason 9 years ago.
apply on top of previous patch

Download all attachments as: .zip

Change History (19)

Changed 9 years ago by ncohen

Changed 9 years ago by jason

apply on top of previous patch

comment:1 Changed 9 years ago by jason

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 9 years ago by jason

  • 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 9 years ago by mvngu

  • Authors set to Nathann Cohen
  • 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:

  1. graph_tour.patch
  2. trac-6774-graph-tour-review.patch

See #6952 for a follow-up to this ticket.

comment:4 follow-up: Changed 9 years ago by jason

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

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 9 years ago by jhpalmieri

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 9 years ago by ncohen

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 9 years ago by ncohen

  • Description modified (diff)
  • Report Upstream set to N/A
  • Resolution fixed deleted
  • Status changed from closed to new

comment:9 Changed 9 years ago by ncohen

  • Status changed from new to needs_review

comment:10 Changed 9 years ago by mhansen

  • Status changed from needs_review to positive_review

comment:11 follow-up: Changed 9 years ago by mhansen

I thought that we didn't want things in the tutorial that depended on optional packages.

comment:12 in reply to: ↑ 11 Changed 9 years ago by mvngu

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

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 9 years ago by ncohen

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 9 years ago by jhpalmieri

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 9 years ago by jhpalmieri

Can we close this as "won't fix"?

comment:17 Changed 9 years ago by mvngu

  • 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.

Note: See TracTickets for help on using tickets.