sagemath/sage

Modularization fixes in sage.graphs

mkoeppe opened this issue · 16 comments

mkoeppe commented

This is for #32601 (sagemath-standard-no-symbolics).

CC: @dcoudert @dimpase

Component: refactoring

Author: Matthias Koeppe

Branch/Commit: 4803982

Reviewer: David Coudert

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

mkoeppe commented

Last 10 new commits:

1c95e82sage.graphs.graph_decompositions.tree_decomposition.treelength_lowerbound: Avoid ceil from sage.functions, remove workaround for python 2 integer div
df49677src/sage/graphs/graph_latex.py: Mark doctests # optional - sage.symbolic
8d7b7cdsrc/sage/graphs/generic_graph_pyx.pyx: In doctest, use math.pi with polar_plot
d062a72src/sage/graphs/matchpoly.pyx: Mark a doctest # optional - sage.symbolic
db13d3fsrc/sage/graphs/bipartite_graph.py: Mark some doctests # optional - sage.symbolic
bab8600src/sage/graphs/generators/families.py: Avoid using sage.functions.trig
7b7b4cdsrc/sage/graphs/graph.py: In doctests, use integer_ceil instead of ceil
d2ada01GenericGraph.laplacian_matrix: Import sqrt only when needed, add # optional - sage.symbolic
a6a3d4csrc/sage/graphs/generators/distance_regular.pyx: Avoid use of sage.functions.trig
9fa5543src/sage/graphs/generic_graph.py: Replace use of symbolic callable expression in doctest by lambda
mkoeppe commented

Commit: 9fa5543

mkoeppe commented

Author: Matthias Koeppe

mkoeppe commented

Description changed:

--- 
+++ 
@@ -1 +1,2 @@
+This is for #32601 (**sagemath-standard-no-symbolics**).
sagetrac-git commented

Changed commit from 9fa5543 to b98d9b3

sagetrac-git commented

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

15b10c1src/sage/graphs/generators/basic.py: In doctest, remove unnecessary use of SR
b98d9b3src/sage/graphs/generators/smallgraphs.py: In doctest, remove unnecessary use of SR
sagetrac-git commented

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

93816acsrc/sage/graphs/graph.py: In doctest, use integer_floor instead of floor
cff9564src/sage/graphs: Mark some doctests # optional - sage.symbolic
1368624src/sage/graphs/generators/families.py: Replace use of floor by //
sagetrac-git commented

Changed commit from b98d9b3 to 1368624

dcoudert commented

Reviewer: David Coudert

dcoudert commented
comment:6

I disagree with this change in generic_graph.py as it changes the label of edges

-            sage: f(x) = -1 / x
-            sage: g(x) = 1 / (x + 1)
+            sage: f = lambda x: -1 / x
+            sage: g = lambda x: 1 / (x + 1)
             sage: G = DiGraph()
             sage: G.add_edges((i, f(i), f) for i in (1, 2, 1/2, 1/4))
             sage: G.add_edges((i, g(i), g) for i in (1, 2, 1/2, 1/4))

The current behavior gives

sage: G.edges()
[(1/4, -4, x |--> -1/x), (1/4, 4/5, x |--> 1/(x + 1)), (1/2, -2, x |--> -1/x), (1/2, 2/3, x |--> 1/(x + 1)), (1, -1, x |--> -1/x), (1, 1/2, x |--> 1/(x + 1)), (2, -1/2, x |--> -1/x), (2, 1/3, x |--> 1/(x + 1))]

which can be nicely converted to latex, while the new behavior gives

sage: G.edges()
[(1/4, -4, <function <lambda> at 0x174ac63a0>), (1/4, 4/5, <function <lambda> at 0x174ac6820>), (1/2, -2, <function <lambda> at 0x174ac63a0>), (1/2, 2/3, <function <lambda> at 0x174ac6820>), (1, -1, <function <lambda> at 0x174ac63a0>), (1, 1/2, <function <lambda> at 0x174ac6820>), (2, -1/2, <function <lambda> at 0x174ac63a0>), (2, 1/3, <function <lambda> at 0x174ac6820>)]

which can obviously not be nicely converted.

All other changes looks good to me.

sagetrac-git commented

Changed commit from 1368624 to 4803982

sagetrac-git commented

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

16d3197Revert "src/sage/graphs/generic_graph.py: Replace use of symbolic callable expression in doctest by lambda"
4803982src/sage/graphs/generic_graph.py: Mark doctest with symbolic edge labels # optional - sage.symbolic
dcoudert commented
comment:9

LGTM.

mkoeppe commented
comment:10

Thanks!