Opened 8 months ago

Closed 8 months ago

#27129 closed enhancement (fixed)

py3: fix other doctests in graph.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: Travis Scrimshaw
Report Upstream: N/A Work issues:
Branch: 40bcb34 (Commits) Commit: 40bcb34762133848f04d3098cef9ae08e1480785
Dependencies: Stopgaps:

Description

We change some doctests in cliques_containing_vertex and perfect_matchings to support py3.

Change History (8)

comment:1 Changed 8 months ago by dcoudert

  • Branch set to public/27129_more_doctests_in_graph
  • Commit set to f8e80ba0fc40c7faf3a7c3519feaec45639c83b8
  • Status changed from new to needs_review

With #27125 and #27127, this should fix all (currently) failing doctests with py3 in file graph.py.


New commits:

f8e80batrac #27129: fix some more doctests in graph.py

comment:2 Changed 8 months ago by tscrim

Can you explain why this change is necessary?

-            sage: G = Graph([[1,-1,'a'], [2,-2, 'b'], [1,-2,'x'], [2,-1,'y']])
+            sage: G = Graph([(0, 1, 'a'), (0, 3, 'b'), (1, 2, 'c'), (2, 3, 'd')])
             sage: list(G.perfect_matchings(labels=True))
-            [[(-2, 1, 'x'), (-1, 2, 'y')], [(-2, 2, 'b'), (-1, 1, 'a')]]
+            [[(0, 1, 'a'), (2, 3, 'd')], [(0, 3, 'b'), (1, 2, 'c')]]

comment:3 Changed 8 months ago by dcoudert

Essentially the same motivation as in https://trac.sagemath.org/ticket/27127#comment:3. The order in which vertices are visited is not always the same in py2 and py3, and so the order in which solutions are discovered may differ.

Here, with py3, we get [[(-1, 1, 'a'), (-2, 2, 'b')], [(-1, 2, 'y'), (-2, 1, 'x')]]. So same set of solutions than with py2, but matchings are reported in a different order, and the order of edges of each matching is different.

The proposed change is more stable with py2 and py3.

An alternative is to do:

sage: def compare(A, B):
....:     return Set(map(Set, A)) == Set(map(Set, B))
sage: G = Graph([[1,-1,'a'], [2,-2, 'b'], [1,-2,'x'], [2,-1,'y']])
sage: expected = [[(-2, 1, 'x'), (-1, 2, 'y')], [(-2, 2, 'b'), (-1, 1, 'a')]]
sage: L = list(G.perfect_matchings(labels=True))
sage: compare(L, expected)
True

Should I do that ?

comment:4 Changed 8 months ago by tscrim

That doesn't explain why this change is necessary. It just seems to replace a brittle test with another brittle test. Why not just run sorted on the output (since everything is a (int, int, str), there should be no invalid comparisons)?

comment:5 Changed 8 months ago by git

  • Commit changed from f8e80ba0fc40c7faf3a7c3519feaec45639c83b8 to 40bcb34762133848f04d3098cef9ae08e1480785

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

e5c60b9trac #27129: Merged with 8.7.beta1
40bcb34trac #27129: fix doctest in perfect matching

comment:6 Changed 8 months ago by dcoudert

I agree. I'm now using sorted twice. This doctest should be stable now.

comment:7 Changed 8 months ago by tscrim

  • Reviewers set to Travis Scrimshaw
  • Status changed from needs_review to positive_review

Thank you.

comment:8 Changed 8 months ago by vbraun

  • Branch changed from public/27129_more_doctests_in_graph to 40bcb34762133848f04d3098cef9ae08e1480785
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.