Opened 2 years ago

Closed 23 months ago

# Merge duplications in edges, ridges and f-vector of combinatorial polyhedron

Reported by: Owned by: gh-kliem major sage-9.3 geometry code duplication, combinatorial polyhedron Jean-Philippe Labbé, Laith Rastanawi, Travis Scrimshaw Jonathan Kliem Travis Scrimshaw N/A 62b3d9f 62b3d9f0eaf22fc9102f990f4c7f3754c1282f79 #30443

### Description

We merge some duplications of obtaining edges, ridges and f-vector.

There was an entire copy of getting edges/ridges that also obtained the f-vector. But it is much simpler doing this directly in the f-vector code.

Also getting the edges/ridges is almost the same thing. We can let the FaceIterator worry about the details.

### comment:1 Changed 2 years ago by gh-kliem

Status: new → needs_review

### comment:2 Changed 2 years ago by Matthias Köppe

Milestone: sage-9.2 → sage-9.3

### comment:3 Changed 2 years ago by gh-kliem

Cc: Jean-Philippe Labbé Laith Rastanawi Travis Scrimshaw added

### comment:4 Changed 2 years ago by Travis Scrimshaw

Reviewers: → Travis Scrimshaw

Some minor formatting things (which you can either change or ignore and then set this to a positive review):

-    cdef size_t _compute_edges_or_ridges_with_iterator(self, FaceIterator face_iter, bint do_atom_rep, size_t ***edges_pt, size_t *counter_pt, size_t *current_length_pt, MemoryAllocator mem) except -1:
+    cdef size_t _compute_edges_or_ridges_with_iterator(self, FaceIterator face_iter,
+                                                       bint do_atom_rep,
+                                                       size_t ***edges_pt,
+                                                       size_t *counter_pt,
+                                                       size_t *current_length_pt,
+                                                       MemoryAllocator mem) except -1:
r"""
See :meth:`CombinatorialPolyhedron._compute_edges`.
"""
cdef size_t a,b                # facets of an edge
cdef int dim = self.dimension()
cdef output_dimension = 1 if do_atom_rep else dim - 2

-        while (face_iter.next_dimension() == output_dimension):
+        while face_iter.next_dimension() == output_dimension:

### comment:5 Changed 2 years ago by git

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

 ​cbc711d Merge branch 'u/gh-kliem/merge_edges_ridges_f_vector' of git://trac.sagemath.org/sage into u/gh-kliem/merge_edges_ridges_f_vector ​62b3d9f formating

### comment:6 Changed 2 years ago by gh-kliem

Status: needs_review → positive_review

Thank you.

### comment:7 Changed 23 months ago by Volker Braun

Branch: u/gh-kliem/merge_edges_ridges_f_vector → 62b3d9f0eaf22fc9102f990f4c7f3754c1282f79 → fixed positive_review → closed
Note: See TracTickets for help on using tickets.