Opened 4 years ago

Closed 4 years ago

#27129 closed enhancement (fixed)

py3: fix other doctests in graph.py

Reported by: David Coudert 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, GitHub, GitLab) Commit: 40bcb34762133848f04d3098cef9ae08e1480785
Dependencies: Stopgaps:

Status badges

Description

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

Change History (8)

comment:1 Changed 4 years ago by David Coudert

Branch: public/27129_more_doctests_in_graph
Commit: f8e80ba0fc40c7faf3a7c3519feaec45639c83b8
Status: newneeds_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 4 years ago by Travis Scrimshaw

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 4 years ago by David Coudert

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 4 years ago by Travis Scrimshaw

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 4 years ago by git

Commit: f8e80ba0fc40c7faf3a7c3519feaec45639c83b840bcb34762133848f04d3098cef9ae08e1480785

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 4 years ago by David Coudert

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

comment:7 Changed 4 years ago by Travis Scrimshaw

Reviewers: Travis Scrimshaw
Status: needs_reviewpositive_review

Thank you.

comment:8 Changed 4 years ago by Volker Braun

Branch: public/27129_more_doctests_in_graph40bcb34762133848f04d3098cef9ae08e1480785
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.