sagemath/sage

Polyhedron_base.vertex_adjacency_matrix: Do not use face_lattice

Closed this issue · 14 comments

instead just go through self.combinatorial_polyhedron().edges()

This should be faster - and only avoids an indirect dependency of sage.geometry.polyhedron.plot on sage.combinat

CC: @kliem

Component: geometry

Author: Jonathan Kliem

Branch/Commit: 2cd253c

Reviewer: Frédéric Chapoton, Matthias Koeppe

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

kliem commented

New commits:

7db923ecompute vertex adjacency matrix from edges
21a6c94remove code duplications
kliem commented

Author: Jonathan Kliem

kliem commented

Commit: 21a6c94

kliem commented

Branch: public/32666

Reviewer: Frédéric Chapoton

comment:2

ok, looks good

comment:3

Thanks for working on this!

kliem commented
comment:4

Thanks for the review.
This change turned out to be surprisingly uncomplicated.

comment:5

how about

--- a/src/sage/geometry/polyhedron/combinatorial_polyhedron/base.pyx
+++ b/src/sage/geometry/polyhedron/combinatorial_polyhedron/base.pyx
@@ -1276,7 +1276,7 @@ cdef class CombinatorialPolyhedron(SageObject):
             sage: polytopes.cube().vertex_adjacency_matrix().is_immutable()
             True
         """
-        from sage.rings.all import ZZ
+        from sage.rings.integer_ring import ZZ
         from sage.matrix.constructor import matrix

Changed commit from 21a6c94 to 2cd253c

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:

2cd253cmore specific import

Changed reviewer from Frédéric Chapoton to Frédéric Chapoton, Matthias Koeppe

comment:7

Thanks!

Changed branch from public/32666 to 2cd253c