Opened 5 months ago

Closed 4 months ago

#27170 closed enhancement (fixed)

py3: fix 14 doctests in digraph.py

Reported by: dcoudert Owned by:
Priority: major Milestone: sage-8.7
Component: graph theory Keywords: py3, graph
Cc: Merged in:
Authors: David Coudert Reviewers: Vincent Klein
Report Upstream: N/A Work issues:
Branch: 0ab4c58 (Commits) Commit: 0ab4c583d2f73d087afd3b45c04ad7e2a5e5f77f
Dependencies: Stopgaps:

Description

Some doctests are now marked as py2 or py3 as the result depends on internal ordering of vertices.

For the doctests of methods _all_paths_iterator and all_paths_iterator, it seems hard to make similar doctest in py3 than the tests in py2. Again, the result depends on some internal ordering and the result in py3 is unstable. Also we mark some existing doctest as py2and add new doctests that work with both py2 and py3.

Change History (9)

comment:1 Changed 5 months ago by dcoudert

  • Branch set to public/27170_digraph
  • Commit set to c66c2b90f2775a621955cdb13967491747148fb7
  • Status changed from new to needs_review

New commits:

c66c2b9trac #27170: fix doctests in digraph.py

comment:2 Changed 4 months ago by vklein

Hi ! I think we can use less #py2, #py3 for this ticket. Proposal for in_degree_iterator calls :

diff --git a/src/sage/graphs/digraph.py b/src/sage/graphs/digraph.py
index 2066cfa..6e43c34 100644
--- a/src/sage/graphs/digraph.py
+++ b/src/sage/graphs/digraph.py
@@ -1274,26 +1274,17 @@ class DiGraph(GenericGraph):
         EXAMPLES::
 
             sage: D = graphs.Grid2dGraph(2,4).to_directed()
-            sage: for i in D.in_degree_iterator():
-            ....:     print(i)
-            3
-            3
-            2
-            2
-            3
-            2
-            2
-            3
-            sage: for i in D.in_degree_iterator(labels=True):
-            ....:    print(i)
-            ((0, 1), 3)
-            ((1, 2), 3)
-            ((0, 0), 2)
-            ((0, 3), 2)
-            ((1, 1), 3)
-            ((1, 3), 2)
-            ((1, 0), 2)
-            ((0, 2), 3)
+            sage: sorted(D.in_degree_iterator())
+            [2, 2, 2, 2, 3, 3, 3, 3]
+            sage: sorted(D.in_degree_iterator(labels=True))
+            [((0, 0), 2),
+             ((0, 1), 3),
+             ((0, 2), 3),
+             ((0, 3), 2),
+             ((1, 0), 2),
+             ((1, 1), 3),
+             ((1, 2), 3),
+             ((1, 3), 2)]
         """
         if vertices is None:
             vertices = self.vertex_iterator()

comment:3 Changed 4 months ago by git

  • Commit changed from c66c2b90f2775a621955cdb13967491747148fb7 to 0a4cee9ea79f53e08a10d7cbb80b6e4e9541c3aa

Branch pushed to git repo; I updated commit sha1. New commits:

43a8851trac #27170: Merged with 8.7.beta5
0a4cee9trac #27170: doctests without py2/py3 tags

comment:4 Changed 4 months ago by dcoudert

I changed the doctests to remove py2 and py3 tags. I also changed so tests to avoid the use od Counter.

I changed the way to declare the digraph in the doctest with the hope that it could be more stable over py2/py3... At least all doctests pass on my side.

comment:5 Changed 4 months ago by vklein

I get the fololwing errors with python3:

sage -t --long src/sage/graphs/digraph.py 
too many failed tests, not using stored timings
Running doctests with ID 2019-03-01-09-49-40-10e3bb29.
Git branch: t/27170/public/27170_digraph
Using --optional=dochtml,gmpy2,memlimit,mpir,python2,sage
Doctesting 1 file.
sage -t --long src/sage/graphs/digraph.py
**********************************************************************
File "src/sage/graphs/digraph.py", line 2112, in sage.graphs.digraph.DiGraph.?
Failed example:
    for _ in range(5):
        print(next(pi))
Expected:
    ['a', 'a']
    ['a', 'b']
    ['a', 'a', 'a']
    ['a', 'a', 'b']
    ['a', 'b', 'c']
Got:
    ['a', 'b']
    ['a', 'a']
    ['a', 'b', 'c']
    ['a', 'a', 'b']
    ['a', 'a', 'a']
**********************************************************************
File "src/sage/graphs/digraph.py", line 2149, in sage.graphs.digraph.DiGraph.?
Failed example:
    for _ in range(5):
        print(next(pi))
Expected:
    ['a', 'b', 'c']
    ['a', 'a', 'b', 'c']
    ['a', 'a', 'a', 'b', 'c']
    ['a', 'b', 'c', 'd', 'c']
    ['a', 'a', 'a', 'a', 'b', 'c']
Got:
    ['a', 'b', 'c']
    ['a', 'a', 'b', 'c']
    ['a', 'b', 'c', 'd', 'c']
    ['a', 'a', 'a', 'b', 'c']
    ['a', 'a', 'b', 'c', 'd', 'c']
**********************************************************************
File "src/sage/graphs/digraph.py", line 2160, in sage.graphs.digraph.DiGraph.?
Failed example:
    for _ in range(5):
        print(next(pi))
Expected:
    ['a', 'a']
    ['a', 'b']
    ['a', 'a', 'a']
    ['a', 'a', 'b']
    ['a', 'a', 'a', 'a']
Got:
    ['a', 'b']
    ['a', 'a']
    ['a', 'a', 'b']
    ['a', 'a', 'a']
    ['a', 'a', 'a', 'b']
**********************************************************************
File "src/sage/graphs/digraph.py", line 2180, in sage.graphs.digraph.DiGraph.?
Failed example:
    list(pi)
Expected:
    [['a'], ['a', 'a'], ['a', 'b'], ['a', 'a', 'a'], ['a', 'a', 'b'],
     ['a', 'b', 'c'], ['a', 'a', 'a', 'a'], ['a', 'a', 'a', 'b'],
     ['a', 'a', 'b', 'c'], ['a', 'b', 'c', 'd']]
Got:
    [['a'],
     ['a', 'b'],
     ['a', 'a'],
     ['a', 'b', 'c'],
     ['a', 'a', 'b'],
     ['a', 'a', 'a'],
     ['a', 'b', 'c', 'd'],
     ['a', 'a', 'b', 'c'],
     ['a', 'a', 'a', 'b'],
     ['a', 'a', 'a', 'a']]
**********************************************************************
File "src/sage/graphs/digraph.py", line 2277, in sage.graphs.digraph.DiGraph.?
Failed example:
    for _ in range(7):
        print(next(pi))
Expected:
    ['a', 'a']
    ['a', 'b']
    ['b', 'c']
    ['c', 'd']
    ['d', 'c']
    ['a', 'a', 'a']
    ['a', 'a', 'b']
Got:
    ['a', 'b']
    ['a', 'a']
    ['b', 'c']
    ['c', 'd']
    ['d', 'c']
    ['a', 'b', 'c']
    ['a', 'a', 'b']
**********************************************************************
File "src/sage/graphs/digraph.py", line 2293, in sage.graphs.digraph.DiGraph.?
Failed example:
    for _ in range(5):
        print(next(pi))
Expected:
    ['a', 'a']
    ['a', 'b']
    ['a', 'a', 'a']
    ['a', 'a', 'b']
    ['a', 'b', 'c']
Got:
    ['a', 'b']
    ['a', 'a']
    ['a', 'b', 'c']
    ['a', 'a', 'b']
    ['a', 'a', 'a']
**********************************************************************
File "src/sage/graphs/digraph.py", line 2324, in sage.graphs.digraph.DiGraph.?
Failed example:
    list(pi)
Expected:
    [['a', 'b'], ['a', 'a', 'b'], ['a', 'b', 'c'],
     ['a', 'a', 'a', 'b'], ['a', 'a', 'b', 'c'],
     ['a', 'a', 'a', 'a', 'b'], ['a', 'a', 'a', 'b', 'c'],
     ['a', 'b', 'c', 'd', 'c'], ['a', 'a', 'a', 'a', 'a', 'b'],
     ['a', 'a', 'a', 'a', 'b', 'c'], ['a', 'a', 'b', 'c', 'd', 'c'],
     ['a', 'a', 'a', 'a', 'a', 'a', 'b'],
     ['a', 'a', 'a', 'a', 'a', 'b', 'c'],
     ['a', 'a', 'a', 'b', 'c', 'd', 'c'],
     ['a', 'b', 'c', 'd', 'c', 'd', 'c']]
Got:
    [['a', 'b'],
     ['a', 'b', 'c'],
     ['a', 'a', 'b'],
     ['a', 'a', 'b', 'c'],
     ['a', 'a', 'a', 'b'],
     ['a', 'b', 'c', 'd', 'c'],
     ['a', 'a', 'a', 'b', 'c'],
     ['a', 'a', 'a', 'a', 'b'],
     ['a', 'a', 'b', 'c', 'd', 'c'],
     ['a', 'a', 'a', 'a', 'b', 'c'],
     ['a', 'a', 'a', 'a', 'a', 'b'],
     ['a', 'b', 'c', 'd', 'c', 'd', 'c'],
     ['a', 'a', 'a', 'b', 'c', 'd', 'c'],
     ['a', 'a', 'a', 'a', 'a', 'b', 'c'],
     ['a', 'a', 'a', 'a', 'a', 'a', 'b']]
**********************************************************************
1 item had failures:
   7 of 150 in sage.graphs.digraph.DiGraph.?
    [478 tests, 7 failures, 4.34 s]
----------------------------------------------------------------------
sage -t --long src/sage/graphs/digraph.py  # 7 doctests failed
----------------------------------------------------------------------
Total time for all tests: 4.4 seconds
    cpu time: 4.1 seconds
    cumulative wall time: 4.3 seconds

Apparently it's from the tests based on _all_paths_iterator.
As sorting will mean build a long list it will be costly (very long test). I think keeping #py2 #py3 tags is good compromise for these cases. Unless of course if you have a solution to have _all_paths_iterator output in the same order for py2 and py3 without hurting performance.

The doctests cases list(pi) can be replaced with sorted(pi)

Last edited 4 months ago by vklein (previous) (diff)

comment:6 Changed 4 months ago by git

  • Commit changed from 0a4cee9ea79f53e08a10d7cbb80b6e4e9541c3aa to 0ab4c583d2f73d087afd3b45c04ad7e2a5e5f77f

Branch pushed to git repo; I updated commit sha1. New commits:

0ab4c58trac #27170: reuse # py2 tags and add sorted

comment:7 Changed 4 months ago by dcoudert

I have marked some doctests as # py2. I don't know how to make equivalent tests for # py3 as for the moment the ordering changes from time to time. May be the use of Python3.7 will help.

I changed the doctests with list(pi). Not the for some of them I added a sort key.

Last, I did a very minor speedup improvement in _all_paths_iterator. It's better to check membership to a graph or a set than to a list.

comment:8 Changed 4 months ago by vklein

  • Reviewers set to Vincent Klein
  • Status changed from needs_review to positive_review

Ok nice ! Looks good to me.

comment:9 Changed 4 months ago by vbraun

  • Branch changed from public/27170_digraph to 0ab4c583d2f73d087afd3b45c04ad7e2a5e5f77f
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.