Opened 9 years ago

Closed 9 years ago

#14805 closed enhancement (fixed)

Adds sage.graphs.base.graph_backend to the documentation

Reported by: Nathann Cohen Owned by: jason, ncohen, rlm
Priority: trivial Milestone: sage-5.12
Component: graph theory Keywords:
Cc: Merged in: sage-5.12.beta0
Authors: Nathann Cohen Reviewers: Punarbasu Purkayastha
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description (last modified by Punarbasu Purkayastha)

As the title says ! This patch does nothing smart.

Nathann


Apply to devel/sage: trac_14805.patch

Attachments (1)

trac_14805.patch (29.6 KB) - added by Nathann Cohen 9 years ago.

Download all attachments as: .zip

Change History (20)

comment:1 Changed 9 years ago by Nathann Cohen

Status: newneeds_review

comment:2 Changed 9 years ago by Nathann Cohen

Priority: majortrivial

comment:3 Changed 9 years ago by Punarbasu Purkayastha

Patchbot says "mmmm... tasty but can't digest; need rebase."

Edit: That's evil. You upload a new patch in the few seconds while I write. X-(

Last edited 9 years ago by Punarbasu Purkayastha (previous) (diff)

comment:4 Changed 9 years ago by Nathann Cohen

Sorrysorry I just did ! ;-)

Nathann

comment:5 Changed 9 years ago by Punarbasu Purkayastha

Can you add the following:

  1. After every description iterator, add what it is an iterator of. In some cases it is clearly mentioned, whereas in other cases it is simply left as iterator.
  2. There are some INPUT descriptions like this: - ``new`` -- boolean or None or - ``new`` -- string or None . I think it should be explicitly mentioned that None corresponds to retrieving the current value. In fact, I am very surprised how it works. The function definition should be like the definition below so that G.loops() gives the value straightaway, without having to enter None as a function argument:
    def loops(self, new=None):
    

It is a painful and extensive cleanup of that file. Thanks for the effort. :)

comment:6 Changed 9 years ago by Punarbasu Purkayastha

  1. There is this sentence which needs spaces around the =, otherwise the rendered output is weird.
    81	        If ``name``=``None``, the new vertex name is returned, ``None`` otherwise.
    
    784	        If ``name``=``None``, the new vertex name is returned. ``None`` otherwise.
    
  2. This needs a double backticks; currently it is rendered in latex
    201	            label of `(u,v)`
    
  3. It applied with one hunk at fuzz 2 against 5.11.beta3.

comment:7 Changed 9 years ago by Nathann Cohen

Hellooooooo !

I just updated the patch (sorry for the delay, I had some problems with Sage because of #14737), and fixed all your points.

Except point 4, where I removed the backticks instead of adding them : the whole file is full of (u,v) without backticks, and to me marking them with double backticks makes less sense than writing them as LaTeX characters. To me an edge is a math thing.

Well, what do you think ? We can make it Python, Maths, or let it stay like that too :-)

Nathann

comment:8 Changed 9 years ago by Punarbasu Purkayastha

Only one correction to suggest. In the correction to point 2, the `None` should not be under single backticks. I am ok with removing the backticks from (u,v).

comment:9 Changed 9 years ago by Punarbasu Purkayastha

Oh sorry. there is another one of the point 2 type.

1362	        - ``new`` -- string or None 

comment:10 Changed 9 years ago by Nathann Cohen

Gloops. Right. Patch updated !

Nathann

Changed 9 years ago by Nathann Cohen

Attachment: trac_14805.patch added

comment:11 Changed 9 years ago by Punarbasu Purkayastha

Description: modified (diff)
Reviewers: Punarbasu Purkayastha
Status: needs_reviewpositive_review

Thanks. Looks good to me. :)

comment:12 Changed 9 years ago by Nathann Cohen

Thanks for the review :-)

Nathann

comment:13 Changed 9 years ago by Punarbasu Purkayastha

Why is the patchbot unable to apply to 5.11b3? I applied to 5.11b3 and it worked except for the fuzz.

comment:14 Changed 9 years ago by Nathann Cohen

It is probably an old version. The one I uploaded when you quoted the patchbot, yesterday.

Nathann

comment:15 Changed 9 years ago by Punarbasu Purkayastha

Let's kick the patchbot.

Apply trac_14805.patch

comment:16 Changed 9 years ago by Nathann Cohen

Underfed, then kicked.

We really should treat it better.

Nathann

comment:17 in reply to:  16 ; Changed 9 years ago by Punarbasu Purkayastha

Replying to ncohen:

Underfed, then kicked.

We really should treat it better.

Nathann

"C'est la vie" :-P

comment:18 in reply to:  17 Changed 9 years ago by Nathann Cohen

"C'est la vie" :-P

In life you have to kick back if people treat you like that :-P

Nathann

comment:19 Changed 9 years ago by Jeroen Demeyer

Merged in: sage-5.12.beta0
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.