Opened 6 years ago

Closed 6 years ago

#13784 closed defect (fixed)

Move methods from GenericGraph to Graph

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

Description (last modified by ncohen)

From Sage's reference manual :

"Graph stuff that should not be in this file"

This patch solves the problem :

Apply:

Attachments (2)

trac_13784.patch (44.8 KB) - added by ncohen 6 years ago.
trac_13784-bugfix.patch (1.9 KB) - added by ncohen 6 years ago.

Download all attachments as: .zip

Change History (11)

Changed 6 years ago by ncohen

comment:1 Changed 6 years ago by ncohen

  • Status changed from new to needs_review

comment:2 Changed 6 years ago by dcoudert

  • Reviewers set to David Coudert
  • Status changed from needs_review to needs_work

Some updates are needed.

sage -t  "devel/sage-myclone/sage/graphs/graph.py"          
IBM ILOG License Manager: "IBM ILOG Optimization Suite for Academic Initiative" is accessing CPLEX 12 with option(s): "e m b q ".
**********************************************************************
File "/path-to-sage/sage-5.5.rc0/devel/sage-myclone/sage/graphs/graph.py", line 4904:
    sage: cores=b.cores(with_labels=True); cores
Exception raised:
    Traceback (most recent call last):
      File "/path-to-sage/sage-5.5.rc0/local/bin/ncadoctest.py", line 1231, in run_one_test
        self.run_one_example(test, example, filename, compileflags)
      File "/path-to-sage/sage-5.5.rc0/local/bin/sagedoctest.py", line 38, in run_one_example
        OrigDocTestRunner.run_one_example(self, test, example, filename, compileflags)
      File "/path-to-sage/sage-5.5.rc0/local/bin/ncadoctest.py", line 1172, in run_one_example
        compileflags, 1) in test.globs
      File "<doctest __main__.example_46[7]>", line 1, in <module>
        cores=b.cores(with_labels=True); cores###line 4904:
    sage: cores=b.cores(with_labels=True); cores
    AttributeError: 'DiGraph' object has no attribute 'cores'
**********************************************************************
File "/path-to-sage/sage-5.5.rc0/devel/sage-myclone/sage/graphs/graph.py", line 4906:
    sage: [v for v,c in cores.items() if c>=2] # the vertices in the 2-core
Exception raised:
    Traceback (most recent call last):
      File "/path-to-sage/sage-5.5.rc0/local/bin/ncadoctest.py", line 1231, in run_one_test
        self.run_one_example(test, example, filename, compileflags)
      File "/path-to-sage/sage-5.5.rc0/local/bin/sagedoctest.py", line 38, in run_one_example
        OrigDocTestRunner.run_one_example(self, test, example, filename, compileflags)
      File "/path-to-sage/sage-5.5.rc0/local/bin/ncadoctest.py", line 1172, in run_one_example
        compileflags, 1) in test.globs
      File "<doctest __main__.example_46[8]>", line 1, in <module>
        [v for v,c in cores.items() if c>=Integer(2)] # the vertices in the 2-core###line 4906:
    sage: [v for v,c in cores.items() if c>=2] # the vertices in the 2-core
    NameError: name 'cores' is not defined
**********************************************************************
1 items had failures:
   2 of  13 in __main__.example_46
***Test Failed*** 2 failures.
For whitespace errors, see the file /home/dcoudert/.sage//tmp/graph_29310.py
         [17.6 s]
 
----------------------------------------------------------------------
The following tests failed:


        sage -t  "devel/sage-myclone/sage/graphs/graph.py"
Total time for all tests: 17.6 seconds

comment:3 Changed 6 years ago by ncohen

  • Status changed from needs_work to needs_review

Weird O_o

I read the source code again, but the conclusion is the same : the cores method is not meant to be used on DiGraph?... I mean : in this situation, it returns the same result as G.cores() on the underlying graph.

I just added another patch that changes this DiGraph to a Graph, and tests pass afterwards. I do this in another patch so that the first one only moves methods around without modifying them. Thanks for noticing this !!!!

Nathann

comment:4 Changed 6 years ago by ncohen

  • Description modified (diff)

comment:5 Changed 6 years ago by dcoudert

The patch is OK, and it passes successfully tests on the entire graph directory.

However, when looking at the doc, I saw a very small bug that is not due to this patch:

    :meth=`~GenericGraph.is_cut_edge` | Returns True if the input edge is a cut-edge or a bridge. 

Could you fix this missing ":" after ":meth" in this patch, or is it better to have a dedicated patch?

comment:6 Changed 6 years ago by ncohen

No you are right, I already forgot many times to fix it somewhere ^^;

Give me ten seconds :-P

Nathann

comment:7 Changed 6 years ago by ncohen

Here it is !

Nathann

Changed 6 years ago by ncohen

comment:8 Changed 6 years ago by dcoudert

  • Authors set to Nathann Cohen
  • Status changed from needs_review to positive_review

Thanks.

comment:9 Changed 6 years ago by jdemeyer

  • Merged in set to sage-5.6.beta0
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.