Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix tests with networkx 2.7 #33495

Closed
antonio-rojas opened this issue Mar 13, 2022 · 14 comments
Closed

Fix tests with networkx 2.7 #33495

antonio-rojas opened this issue Mar 13, 2022 · 14 comments

Comments

@antonio-rojas
Copy link
Contributor

The output of matching displays edges in reverse order in 2.7, which break tests. To fix it, we make the output of matching and Edgesview, which also fixes some other issues, such as:

sage: G=graphs.CompleteGraph(4)
sage: m=G.matching(); m
[(1, 2, None), (0, 3, None)]
sage: (3, 0, None) in m
False

Also, networkx.node_clique_number now outputs a collections.defaultdict object, so we explicitly cast it to dict for backwards compatibility.

CC: @mkoeppe @kiwifb @tornaria

Component: graph theory

Author: Antonio Rojas

Branch/Commit: 8452003

Reviewer: David Coudert

Issue created by migration from https://trac.sagemath.org/ticket/33495

@antonio-rojas antonio-rojas added this to the sage-9.6 milestone Mar 13, 2022
@antonio-rojas
Copy link
Contributor Author

Branch: u/arojas/fix_tests_with_networkx_2_7

@antonio-rojas
Copy link
Contributor Author

Commit: 66a1fe8

@antonio-rojas
Copy link
Contributor Author

Author: Antonio Rojas

@antonio-rojas
Copy link
Contributor Author

New commits:

c6dc11bUse EdgesView for matching output
66a1fe8Cast networkx.node_clique_number output to dict

@antonio-rojas

This comment has been minimized.

@antonio-rojas

This comment has been minimized.

@dcoudert
Copy link
Contributor

comment:4

You should

  • specify the input format to Graph to reduce the overhead of building the graph before returning the result.
-                return EdgesView(Graph([(u, v, L[frozenset((u, v))]) for u, v in d]))
+                return EdgesView(Graph([(u, v, L[frozenset((u, v))]) for u, v in d], format='list_of_edges'))
  • update the documentation to clarify that the output is now a EdgesView
  • modify a doctest to show that the result is of type EdgesView, for instance like
-            sage: sorted(g.matching(use_edge_labels=True))
-            [(0, 3, 3), (1, 2, 6)]
+            sage: m = g.matching(use_edge_labels=True)
+            sage: type(m)
+            <class 'sage.graphs.views.EdgesView'>
+            sage: sorted(m)
+            [(0, 3, 3), (1, 2, 6)]

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 14, 2022

Changed commit from 66a1fe8 to 8452003

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 14, 2022

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

2a7fa3cSpecify the input format to Graph
8452003Update tests and docs

@antonio-rojas
Copy link
Contributor Author

comment:6

Done, thanks

@dcoudert
Copy link
Contributor

comment:7

LGTM.

@dcoudert
Copy link
Contributor

Reviewer: David Coudert

@mkoeppe
Copy link
Member

mkoeppe commented Mar 17, 2022

comment:8

Follow-up: #33520

@vbraun
Copy link
Member

vbraun commented Mar 27, 2022

Changed branch from u/arojas/fix_tests_with_networkx_2_7 to 8452003

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants