Skip to content

Conversation

fchapoton
Copy link
Contributor

@fchapoton fchapoton commented Nov 16, 2023

fix many small details in pyx files in graphs, as found by

cython-lint --ignore=E501,E741,E265,E221,E231,E201,E127,E129 src/sage/graphs/

notably removing many unused def and variables.

There remains in particular a rather suspicious warning:

src/sage/graphs/distances_all_pairs.pyx:1977:9: 'ecc_antipode' defined but unused (try prefixing with underscore?)

📝 Checklist

  • The title is concise, informative, and self-explanatory.
  • The description explains in detail what this PR is about.

@fchapoton fchapoton requested a review from dcoudert November 16, 2023 19:14
Copy link

Documentation preview for this PR (built with commit 4920ae7; changes) is ready! 🎉

Copy link
Contributor

@dcoudert dcoudert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@fchapoton
Copy link
Contributor Author

merci. Quid du truc sur "ecc_antipode" défini mais pas utilisé ?

@dcoudert
Copy link
Contributor

merci. Quid du truc sur "ecc_antipode" défini mais pas utilisé ?

Indeed, in file src/sage/graphs/distances_all_pairs.pyx, method radius_DHV, we can do:

diff --git a/src/sage/graphs/distances_all_pairs.pyx b/src/sage/graphs/distances_all_pairs.pyx
index a61fd4168a..412c1b51cb 100644
--- a/src/sage/graphs/distances_all_pairs.pyx
+++ b/src/sage/graphs/distances_all_pairs.pyx
@@ -1940,7 +1940,7 @@ def radius_DHV(G):
     init_short_digraph(sd, G, edge_labelled=False, vertex_list=int_to_vertex)
 
     cdef uint32_t source, ecc_source
-    cdef uint32_t antipode, ecc_antipode
+    cdef uint32_t antipode
     cdef uint32_t UB = UINT32_MAX
     cdef uint32_t LB = 0
 
@@ -1975,9 +1975,9 @@ def radius_DHV(G):
 
         # 2) Take vertex at largest distance from source, called antipode (last
         # vertex visited in simple_BFS), and compute its BFS distances.
-        # By definition of antipode, we have ecc_antipode >= ecc_source.
+        # By definition of antipode, we have ecc(antipode) >= ecc(source).
         antipode = waiting_list[n-1]
-        ecc_antipode = simple_BFS(sd, antipode, distances, NULL, waiting_list, seen)
+        _ = simple_BFS(sd, antipode, distances, NULL, waiting_list, seen)
 
         # 3) Use distances from antipode to improve eccentricity lower bounds.
         # We also determine the next source

@vbraun vbraun merged commit 9938ca9 into sagemath:develop Dec 10, 2023
@fchapoton fchapoton deleted the cylint-for-graphs branch December 10, 2023 14:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants