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

clean generic_graph.py (part 9) - edge and vertex handlers #26666

Closed
dcoudert opened this issue Nov 9, 2018 · 17 comments
Closed

clean generic_graph.py (part 9) - edge and vertex handlers #26666

dcoudert opened this issue Nov 9, 2018 · 17 comments

Comments

@dcoudert
Copy link
Contributor

dcoudert commented Nov 9, 2018

In this ticket, we clean methods for adding/removing vertices/edges and enumerating neighbors. The main changes are:

  • add parameter sort to method .vertices. This gives an alternative to list(G).
  • in method delete_vertices, we add as first instruction vertices = list(vertices). This way we now allow to call this method with an iterator. Before, the method was not removing any vertex when called with an iterator. This will certainly help reducing further the number of calls to .vertices().
  • in method vertex_iterator, we remove doctests related to former networkx backend
  • change in method remove_multiple_edges that should make it faster
  • PEP8 cleaning

CC: @tscrim @fchapoton

Component: graph theory

Author: David Coudert

Branch/Commit: 1c43e4e

Reviewer: Travis Scrimshaw

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

@dcoudert dcoudert added this to the sage-8.5 milestone Nov 9, 2018
@dcoudert
Copy link
Contributor Author

dcoudert commented Nov 9, 2018

Commit: 0070906

@dcoudert
Copy link
Contributor Author

dcoudert commented Nov 9, 2018

@dcoudert
Copy link
Contributor Author

dcoudert commented Nov 9, 2018

New commits:

0070906trac #26666: generic_graph.py (part 9) - edge and vertex handlers

@fchapoton
Copy link
Contributor

comment:2

2 trivial failing doctests in src/sage/graphs/bipartite_graph.py

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 9, 2018

Changed commit from 0070906 to ecfc5c6

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 9, 2018

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

7131aaatrac #26666: fix doc
ecfc5c6trac #26666: fix doctests in bipartite_graph.py

@dcoudert
Copy link
Contributor Author

dcoudert commented Nov 9, 2018

comment:4

oups, fixed.

@dcoudert dcoudert changed the title clean connectivity.pyx (part 9) - edge and vertex handlers clean generic_graph.py (part 9) - edge and vertex handlers Nov 10, 2018
@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 10, 2018

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

44c76f2trac #26666: avoid .vertices() in method clear

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 10, 2018

Changed commit from ecfc5c6 to 44c76f2

@dcoudert
Copy link
Contributor Author

comment:7

We also avoid sorting vertices before removing them from the graph.

@tscrim
Copy link
Collaborator

tscrim commented Nov 10, 2018

comment:8

Just for clarity, I think it is better to add these parentheses: if (not sort) and key:

Since u and v are code inputs for the one-line descriptions, I think it would be better to do

-Delete all edges from `u` to `v`.
+Delete all edges from ``u`` to ``v``.

I think the OUTPUT: block for has_vertex is functionally useless and can be safely removed (it is essentially a copy of the one-line description).

Other than that LGTM.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 10, 2018

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

1c43e4etrac #26666: implement reviewer's comments

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 10, 2018

Changed commit from 44c76f2 to 1c43e4e

@dcoudert
Copy link
Contributor Author

comment:10

i have implemented all you comments. Should be better now.

@tscrim
Copy link
Collaborator

tscrim commented Nov 10, 2018

Reviewer: Travis Scrimshaw

@tscrim
Copy link
Collaborator

tscrim commented Nov 10, 2018

comment:11

Thanks. LGTM.

@vbraun
Copy link
Member

vbraun commented Nov 12, 2018

Changed branch from public/26666_generic_graph_part_9_edge_and_vertex to 1c43e4e

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