Skip to content

Commit 44aef4a

Browse files
author
Release Manager
committed
gh-35087: refactor: Use enum for face iterator status <!-- ^^^^^ Please provide a concise, informative and self-explanatory title. Don't put issue numbers in there, do this in the PR body below. For example, instead of "Fixes #1234" use "Introduce new method to calculate 1+1" --> ### 📚 Description Related to the feature request in #34773, I'm trying to make my own code a bit more readable. Here we replace the int representing the status of the face of the iterator by an enumerator. This allows understanding this variable without having to look up the meaning. ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> <!-- If your change requires a documentation PR, please link it appropriately --> <!-- If you're unsure about any of these, don't hesitate to ask. We're here to help! --> - [x] I have made sure that the title is self-explanatory and the description concisely explains the PR. - [x] I have linked an issue or discussion. - [ ] I have created tests covering the changes. - [ ] I have updated the documentation accordingly. ### ⌛ Dependencies <!-- List all open pull requests that this PR logically depends on --> <!-- - #xyz: short description why this is a dependency - #abc: ... --> URL: #35087 Reported by: Jonathan Kliem Reviewer(s): Jonathan Kliem, Matthias Köppe
2 parents 069ac77 + 038fbda commit 44aef4a

File tree

3 files changed

+24
-18
lines changed

3 files changed

+24
-18
lines changed

src/sage/geometry/polyhedron/combinatorial_polyhedron/combinatorial_face.pyx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ import numbers
7070
from sage.rings.integer cimport smallInteger
7171
from .conversions cimport bit_rep_to_Vrep_list
7272
from .base cimport CombinatorialPolyhedron
73-
from .face_iterator cimport FaceIterator_base
73+
from .face_iterator cimport FaceIterator_base, FaceStatus
7474
from .polyhedron_face_lattice cimport PolyhedronFaceLattice
7575
from .face_data_structure cimport face_len_atoms, face_init, face_free, face_copy, face_issubset
7676
from .face_list_data_structure cimport bit_rep_to_coatom_rep
@@ -177,7 +177,7 @@ cdef class CombinatorialFace(SageObject):
177177
self.atoms = it.atoms
178178
self.coatoms = it.coatoms
179179

180-
if it.structure.face_status == 0:
180+
if it.structure.face_status == FaceStatus.NOT_INITIALIZED:
181181
raise LookupError("face iterator not set to a face")
182182

183183
face_init(self.face, self.coatoms.n_atoms(), self.coatoms.n_coatoms())

src/sage/geometry/polyhedron/combinatorial_polyhedron/face_iterator.pxd

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,16 @@ from .face_data_structure cimport face_t
55
from .face_list_data_structure cimport face_list_t
66
from .combinatorial_face cimport CombinatorialFace
77

8+
cdef enum FaceStatus:
9+
NOT_INITIALIZED
10+
INITIALIZED
11+
IGNORE_SUBSETS
12+
ONLY_VISIT_SUBSETS
13+
814
cdef struct iter_s:
915
bint dual # if 1, then iterate over dual Polyhedron
1016
face_t face # the current face of the iterator
11-
int face_status # 0 not initialized, 1 initialized, 2 added to visited_all, 3 only visit subsets
17+
FaceStatus face_status
1218
size_t *atom_rep # a place where atom-representaion of face will be stored
1319
size_t *coatom_rep # a place where coatom-representaion of face will be stored
1420
int current_dimension # dimension of current face, dual dimension if ``dual``

src/sage/geometry/polyhedron/combinatorial_polyhedron/face_iterator.pyx

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -414,7 +414,7 @@ cdef class FaceIterator_base(SageObject):
414414
self.structure.visited_all[self.structure.dimension -1].n_faces = 0
415415
else:
416416
self.structure.visited_all[self.structure.dimension -1].n_faces = 1
417-
self.structure.face_status = 0
417+
self.structure.face_status = FaceStatus.NOT_INITIALIZED
418418
self.structure.new_faces[self.structure.dimension - 1].n_faces = self.coatoms.n_faces()
419419
self.structure.current_dimension = self.structure.dimension - 1
420420
self.structure.highest_dimension = self.structure.dimension - 1
@@ -460,7 +460,7 @@ cdef class FaceIterator_base(SageObject):
460460
sage: next(it).ambient_V_indices() == it.current().ambient_V_indices()
461461
True
462462
"""
463-
if unlikely(self.structure.face_status == 0):
463+
if unlikely(self.structure.face_status == FaceStatus.NOT_INITIALIZED):
464464
raise ValueError("iterator not set to a face yet")
465465
return CombinatorialFace(self)
466466

@@ -888,7 +888,7 @@ cdef class FaceIterator_base(SageObject):
888888
sage: it._meet_of_coatoms(1, 2)
889889
A -1-dimensional face of a Polyhedron in QQ^2
890890
"""
891-
if unlikely(self.structure.face_status != 0):
891+
if unlikely(self.structure.face_status != FaceStatus.NOT_INITIALIZED):
892892
raise ValueError("please reset the face iterator")
893893
if unlikely(self.structure.output_dimension != -2):
894894
raise ValueError("face iterator must not have the output dimension specified")
@@ -994,7 +994,7 @@ cdef class FaceIterator_base(SageObject):
994994
...
995995
IndexError: atoms out of range
996996
"""
997-
if unlikely(self.structure.face_status != 0):
997+
if unlikely(self.structure.face_status != FaceStatus.NOT_INITIALIZED):
998998
raise ValueError("please reset the face iterator")
999999
if unlikely(self.structure.output_dimension != -2):
10001000
raise ValueError("face iterator must not have the output dimension specified")
@@ -1049,14 +1049,14 @@ cdef class FaceIterator_base(SageObject):
10491049
See :meth:`FaceIterator_base.ignore_subfaces` and
10501050
:meth:`FaceIterator_base.ignore_supfaces`.
10511051
"""
1052-
if unlikely(self.structure.face_status == 0):
1052+
if unlikely(self.structure.face_status == FaceStatus.NOT_INITIALIZED):
10531053
raise ValueError("iterator not set to a face yet")
1054-
if unlikely(self.structure.face_status == 3):
1054+
if unlikely(self.structure.face_status == FaceStatus.ONLY_VISIT_SUBSETS):
10551055
# The iterator is consumed, if it was just set to visit only subsets
10561056
# next thing to ignore subsets.
10571057
self.structure.current_dimension = self.structure.dimension
10581058
return 0
1059-
if unlikely(self.structure.face_status == 2):
1059+
if unlikely(self.structure.face_status == FaceStatus.IGNORE_SUBSETS):
10601060
# Nothing to do.
10611061
return 0
10621062
# The current face is added to ``visited_all``.
@@ -1065,7 +1065,7 @@ cdef class FaceIterator_base(SageObject):
10651065
# as there are no new faces.
10661066

10671067
add_face_shallow(self.structure.visited_all[self.structure.current_dimension], self.structure.face)
1068-
self.structure.face_status = 2
1068+
self.structure.face_status = FaceStatus.IGNORE_SUBSETS
10691069

10701070
def only_subfaces(self):
10711071
r"""
@@ -1177,9 +1177,9 @@ cdef class FaceIterator_base(SageObject):
11771177
See :meth:`FaceIterator_base.only_subfaces` and
11781178
:meth:`FaceIterator_base.only_supfaces`.
11791179
"""
1180-
if unlikely(self.structure.face_status == 0):
1180+
if unlikely(self.structure.face_status == FaceStatus.NOT_INITIALIZED):
11811181
raise ValueError("iterator not set to a face yet")
1182-
if unlikely(self.structure.face_status == 2):
1182+
if unlikely(self.structure.face_status == FaceStatus.IGNORE_SUBSETS):
11831183
raise ValueError("cannot only visit subsets after ignoring a face")
11841184

11851185
cdef face_list_t* faces = &self.structure.new_faces[self.structure.current_dimension]
@@ -1191,7 +1191,7 @@ cdef class FaceIterator_base(SageObject):
11911191

11921192
swap_faces(faces[0].faces[yet_to_visit], faces[0].faces[faces[0].n_faces - 1])
11931193

1194-
self.structure.face_status = 3
1194+
self.structure.face_status = FaceStatus.ONLY_VISIT_SUBSETS
11951195
self.structure.yet_to_visit = 0
11961196
# This will work:
11971197
# ``next_dimension`` will first call ``next_face_loop`` and then check
@@ -1281,13 +1281,13 @@ cdef class FaceIterator_base(SageObject):
12811281
if n_atoms == self.coatoms.n_atoms():
12821282
# The face is the universe.
12831283
self.structure.face[0] = face[0]
1284-
self.structure.face_status = 1
1284+
self.structure.face_status = FaceStatus.INITIALIZED
12851285
self.structure.current_dimension = self.structure.dimension
12861286
return 0
12871287
elif n_atoms == 0:
12881288
# The face is the empty face.
12891289
self.structure.face[0] = face[0]
1290-
self.structure.face_status = 1
1290+
self.structure.face_status = FaceStatus.INITIALIZED
12911291
self.structure.current_dimension = -1
12921292
return 0
12931293

@@ -1928,7 +1928,7 @@ cdef inline int next_dimension(iter_t structure, size_t parallelization_depth=0)
19281928
e.g. if it is ``1`` it will stop after having yield all faces of a facet
19291929
"""
19301930
cdef int max_dim = structure.highest_dimension - parallelization_depth
1931-
structure.face_status = 0
1931+
structure.face_status = FaceStatus.NOT_INITIALIZED
19321932
while (not next_face_loop(structure)) and (structure.current_dimension <= max_dim):
19331933
sig_check()
19341934
structure._index += 1
@@ -1959,7 +1959,7 @@ cdef inline int next_face_loop(iter_t structure) nogil except -1:
19591959
# Set ``face`` to the next face.
19601960
structure.yet_to_visit -= 1
19611961
structure.face[0] = faces[0].faces[structure.yet_to_visit][0]
1962-
structure.face_status = 1
1962+
structure.face_status = FaceStatus.INITIALIZED
19631963
return 1
19641964

19651965
if structure.current_dimension <= structure.lowest_dimension:

0 commit comments

Comments
 (0)