Opened 10 years ago

Closed 9 years ago

#5911 closed enhancement (fixed)

greatly improve the documentation one gets from Graph?

Reported by: was Owned by: rlm
Priority: major Milestone: sage-4.2.1
Component: graph theory Keywords:
Cc: Merged in: sage-4.2.1.alpha0
Authors: Nathann Cohen Reviewers: Robert Miller, Minh Van Nguyen, Mike Hansen
Report Upstream: Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description

Imagine a new user who wants to create a graph. They do Graph? and they get (in order):

  1. Two pages of parameters, which they can't possibly read through.
  1. The first *page* of examples all involve networkx (they think -- huh?) and starts like this.
  
    EXAMPLES: We illustrate the first six input formats (the other two
    involve packages that are currently not standard in Sage):
    
    #. A NetworkX XGraph::
    
          sage: import networkx
          sage: g = networkx.XGraph({0:[1,2,3], 2:[4]})
          sage: Graph(g)
          Graph on 5 vertices
....

I propose:

  1. Putting a few simple straightforward examples (which is all most users need) right *before* the INPUT: block.
  1. Moving any mention of networkx lower in the lists, e.g., when defining the data input, don't put networkx first, and when documenting things later with examples, don't put networkx first.
  1. That one can do "graphs.<tab>" and get constructors for any family of graphs should be noted clearly and prominently, also before the INPUT: block. This is not even noted anywhere right now, though it is used in two examples.

The above are all easy changes, I think.

Attachments (4)

doc_graph.patch (5.6 KB) - added by ncohen 9 years ago.
trac_5911-editing.patch (3.4 KB) - added by rlm 9 years ago.
apply on top of doc_graph.patch
trac_5911.patch (8.5 KB) - added by ncohen 9 years ago.
trac_5911-reviewer.patch (10.0 KB) - added by mvngu 9 years ago.
reviewer patch

Download all attachments as: .zip

Change History (23)

comment:1 Changed 10 years ago by jason

+1!

So you're saying that Graph? should be something like graphs?

comment:2 Changed 9 years ago by ncohen

I began to write something, as I thought nobody was working on it seeing that last message was posted 3 months ago ;-)

What I have written is just a short introduction meant for user really unfamiliar with graphs in Sage and in general.. I expect a lot of critics from you, and I will change it accordingly, but I have spent much time in the past days trying to explain how to use graphs in Sage to some of my friends that I thought the best way may be to directly document this section... Tell me what you would change to this :

http://www-sop.inria.fr/members/Nathann.Cohen/infograph.py

comment:3 Changed 9 years ago by ncohen

  • Summary changed from greatly improve the documentation one gets from Graph? to [with draft, needs critics] greatly improve the documentation one gets from Graph?

comment:4 Changed 9 years ago by rlm

Nathann,

This looks pretty good. Can you change the examples a bit, though? For example, a lot of the docstrings about creating graphs from graph6 strings include test cases where an error is triggered. As long as these failures are somewhere in the documentation, they're tested. Maybe the docs here should focus more on how to properly work with them. Also, you should get this into the appropriate place in graph.py and post it as an actual patch, so that e.g. we can post modifications and more people can pitch in to help. Finally, I believe that a few code blocks at the beginning need the :: and indentation, e.g.:

    If you want to see what they look like, begin this way :
   
    sage: g=graphs.PetersenGraph()
    sage: g.plot()

    or

    sage: g=graphs.ChvatalGraph()
    sage: g.plot()

comment:5 Changed 9 years ago by ncohen

Hello !

Several questions :

  • I agree that way to raise errors and exceptions have no real place in Graph?. Where should I put them ? In the looooooooong docstring at the beginning of file graph.py ?
  • I do not understand their purpose anyway. There or elsewhere O_o
  • No problem with '::' as I can see some occurences of them in this docstring... What are they meant to do, though ? ;-)
  • I did not post a patch thinking it would be much easier to read it this way as it seemed far from a final version. The next one will be a patch ;-)

comment:6 Changed 9 years ago by ncohen

Here is the patch you requested.. Where do you think we should store these doctests you mentionned if not in this docstring ?

Changed 9 years ago by ncohen

comment:7 Changed 9 years ago by ncohen

  • Summary changed from [with draft, needs critics] greatly improve the documentation one gets from Graph? to [with draft, needs review] greatly improve the documentation one gets from Graph?

comment:8 Changed 9 years ago by ncohen

  • Summary changed from [with draft, needs review] greatly improve the documentation one gets from Graph? to [with patch, needs review] greatly improve the documentation one gets from Graph?

comment:9 Changed 9 years ago by rlm

  • Summary changed from [with patch, needs review] greatly improve the documentation one gets from Graph? to [with patch, needs work] greatly improve the documentation one gets from Graph?

I've added some editing in a second patch, and I'd like to give this a positive review, but the documentation for DiGraph? suffers from the same fault, and I would feel less guilty if this ticket addressed that as well.

Changed 9 years ago by rlm

apply on top of doc_graph.patch

comment:10 follow-up: Changed 9 years ago by ncohen

What about a good old : "cf. Graph" (or a plain copy of Graph?), as it is exactly the same ? ;

We could just write a list of the functions of DiGraph? that are unaavailable in Graph, couldn't we ?

comment:11 Changed 9 years ago by ncohen

Still around ? ;-)

comment:12 in reply to: ↑ 10 Changed 9 years ago by rlm

Replying to ncohen:

What about a good old : "cf. Graph" (or a plain copy of Graph?), as it is exactly the same ? ;

We could just write a list of the functions of DiGraph? that are unaavailable in Graph, couldn't we ?

Imagine you have never used Sage before, and you really really like DiGraphs?. So one of the first things you do in Sage, aside from 2+2 or factor(factorial(12)), is type DiGraph?. I think there are such (potential) users out there, and the documentation there should be independently helpful. Certainly a reference to Graph? would be appropriate, but the docs you get from DiGraph? should also be self-contained and helpful.

comment:13 follow-up: Changed 9 years ago by ncohen

Then a plain copy could do, couldn't it ? Plus some DiGraph?-specific functions .. ;-)

comment:14 in reply to: ↑ 13 Changed 9 years ago by rlm

Replying to ncohen:

Then a plain copy could do, couldn't it ?

Essentially, yes, subject to s/Graph/DiGraph.

Plus some DiGraph?-specific functions .. ;-)

For the DiGraph? fans!

comment:15 Changed 9 years ago by ncohen

  • Status changed from needs_work to needs_review
  • Summary changed from [with patch, needs work] greatly improve the documentation one gets from Graph? to greatly improve the documentation one gets from Graph?

New version with an updated DiGraph? section. Some problem, still : when running a sage -t on graph.py, Python does not like to see g.diameter ? which is mentioned in the doc.

Do you know how to fix this ?

This new patch is a flattened version of the previous ones, plus the updates. Only this one should be applied.

Nathann

Changed 9 years ago by ncohen

comment:16 Changed 9 years ago by ncohen

Hmmm.. This -- should -- have been a flattened version of all the patches, though it is not and I wonder why. This patch needs doc_graph.patch to be applied first, and contains trac_5911-editing.patch. So only doc_graph.patch and this patch should be applied.

I tried to build a real flattened version, but for some reason I can not O_o

Nathann

Changed 9 years ago by mvngu

reviewer patch

comment:17 Changed 9 years ago by mvngu

I have attached a reviewer patch. So patches should be applied in the following order:

  1. doc_graph.patch
  2. trac_5911.patch
  3. trac_5911-reviewer.patch

The reviewer patch fixes some typos and grammar issues. It also adjusts some examples to prevent doctest failures. Only my patch needs to be reviewed.

comment:18 Changed 9 years ago by mhansen

  • Authors set to Nathann Cohen
  • Reviewers set to Minh Van Nguyen, Mike Hansen
  • Status changed from needs_review to positive_review

Looks good to me.

comment:19 Changed 9 years ago by mhansen

  • Merged in set to sage-4.2.1.alpha0
  • Resolution set to fixed
  • Reviewers changed from Minh Van Nguyen, Mike Hansen to Robert Miller, Minh Van Nguyen, Mike Hansen
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.