Opened 2 years ago

Closed 2 years ago

#26666 closed enhancement (fixed)

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

Reported by: dcoudert Owned by:
Priority: major Milestone: sage-8.5
Component: graph theory Keywords: py3, graph
Cc: tscrim, chapoton Merged in:
Authors: David Coudert Reviewers: Travis Scrimshaw
Report Upstream: N/A Work issues:
Branch: 1c43e4e (Commits, GitHub, GitLab) Commit: 1c43e4ea526f3edb42304c3a170ea402da7b1695
Dependencies: Stopgaps:

Status badges

Description

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

Change History (12)

comment:1 Changed 2 years ago by dcoudert

  • Branch set to public/26666_generic_graph_part_9_edge_and_vertex
  • Cc tscrim chapoton added
  • Commit set to 0070906a5ddd0d72c2f5d082bb70da65744bdd9c
  • Keywords py3 graph added
  • Status changed from new to needs_review

New commits:

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

comment:2 Changed 2 years ago by chapoton

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

comment:3 Changed 2 years ago by git

  • Commit changed from 0070906a5ddd0d72c2f5d082bb70da65744bdd9c to ecfc5c669e49deb9eb0bca906b25764b3f9a8925

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

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

comment:4 Changed 2 years ago by dcoudert

oups, fixed.

comment:5 Changed 2 years ago by dcoudert

  • Summary changed from clean connectivity.pyx (part 9) - edge and vertex handlers to clean generic_graph.py (part 9) - edge and vertex handlers

comment:6 Changed 2 years ago by git

  • Commit changed from ecfc5c669e49deb9eb0bca906b25764b3f9a8925 to 44c76f2e457f94ab0331a09ef6c924a63fd0d60e

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

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

comment:7 Changed 2 years ago by dcoudert

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

comment:8 Changed 2 years ago by tscrim

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.

comment:9 Changed 2 years ago by git

  • Commit changed from 44c76f2e457f94ab0331a09ef6c924a63fd0d60e to 1c43e4ea526f3edb42304c3a170ea402da7b1695

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

​1c43e4etrac #26666: implement reviewer's comments

comment:10 Changed 2 years ago by dcoudert

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

comment:11 Changed 2 years ago by tscrim

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

Thanks. LGTM.

comment:12 Changed 2 years ago by vbraun

  • Branch changed from public/26666_generic_graph_part_9_edge_and_vertex to 1c43e4ea526f3edb42304c3a170ea402da7b1695
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.