Opened 2 years ago

Closed 2 years ago

#28606 closed enhancement (fixed)

CombinatorialPolyhedron: unbounded -> is_bounded

Reported by: gh-kliem Owned by:
Priority: major Milestone: sage-9.0
Component: geometry Keywords: polytopes, combinatorial polyhedron
Cc: jipilab, gh-LaisRast Merged in:
Authors: Jonathan Kliem Reviewers: Laith Rastanawi, Jean-Philippe Labbé
Report Upstream: N/A Work issues:
Branch: c7da39d (Commits, GitHub, GitLab) Commit: c7da39d30e85b8f0e464e4803e1230ad191afc45
Dependencies: #28605 Stopgaps:

Status badges

Description (last modified by gh-kliem)

In order to make CombinatorialPolyhedron to be more consistent with standard naming conventions we replace the method unbounded by is_bounded and the attribute unbounded by bounded.

Change History (19)

comment:1 Changed 2 years ago by gh-kliem

  • Branch set to public/28606
  • Cc jipilab gh-LaisRast added
  • Commit set to 531222d0c562ae412b44e04017459adefef34dd9
  • Status changed from new to needs_review

New commits:

d597ed3replace attributes by methods
2fc4fe0removed empty folder being created in source
37592f9replace attributes by methods; remove empty folder from source
e865f9dremoved attribute Vinv, as its not being used
84ef31badded docstrings to the new methods
588afa4removed method for Vinv
531222dunbounded(self) -> is_bounded(self); _unbounded -> _bounded

comment:2 Changed 2 years ago by git

  • Commit changed from 531222d0c562ae412b44e04017459adefef34dd9 to c8c49c5a173625bcb1529415fb6b6239b32b46c2

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

c8c49c5fixed small mistake in header file

comment:3 Changed 2 years ago by gh-LaisRast

  • Status changed from needs_review to needs_work

Just a very small change: The docstring of is_bounded should now say:

      cdef bint is_bounded(self):
         r"""
-         Return whether the polyhedron is unbounded.
+         Return whether the polyhedron is bounded.
         """
         return self._bounded

comment:4 Changed 2 years ago by git

  • Commit changed from c8c49c5a173625bcb1529415fb6b6239b32b46c2 to 846f2165017f94e9abb4fb30e4be05a8f4df439f

Branch pushed to git repo; I updated commit sha1. Last 10 new commits:

326602cf_vector of CombinatorialPolyhedron is a vector
dfbe2adMerge branch 'public/28607' of git://trac.sagemath.org/sage into public/28621
ed5518bused CombinatorialPolyhedron to compute f_vector
9bdd005give an error message for polytopes in some cases; removed incorrect example
acd671dnow we get a precice error message for inexact truncated dodecahedron
bf85a62subsequent calls for f_vector fail if first attempt fails
dc99ea4Merge branch 'public/28625' of git://trac.sagemath.org/sage into public/28605
9b5bcaaapplied changes of 28605 to new code from 28625
6fb97dcMerge branch 'public/28605' of git://trac.sagemath.org/sage into public/28606
846f216small fix in doc

comment:5 Changed 2 years ago by gh-kliem

  • Status changed from needs_work to needs_review

Actually the last commit is a small fix in doc and applying the changes to #28625.

comment:6 Changed 2 years ago by gh-kliem

  • Dependencies changed from #28605 to #28625, #28605

comment:7 Changed 2 years ago by gh-LaisRast

  • Status changed from needs_review to positive_review

comment:8 Changed 2 years ago by jipilab

  • Status changed from positive_review to needs_work

Missing reviewer name

comment:9 Changed 2 years ago by gh-kliem

  • Reviewers set to Laith Rastanawi
  • Status changed from needs_work to positive_review

comment:10 Changed 2 years ago by jipilab

  • Status changed from positive_review to needs_work

There are failing doctests:

**********************************************************************
File "src/sage/misc/dev_tools.py", line 170, in sage.misc.dev_tools.load_submodules
Failed example:
    sage.misc.dev_tools.load_submodules(sage.geometry)
Expected:
    load sage.geometry.polyhedron.lattice_euclidean_group_element... succeeded
    load sage.geometry.polyhedron.palp_database... succeeded
    load sage.geometry.polyhedron.ppl_lattice_polygon... succeeded
Got:
    load sage.geometry.polyhedron.combinatorial_polyhedron.bit_vector_operations...failed
    load sage.geometry.polyhedron.lattice_euclidean_group_element... succeeded
    load sage.geometry.polyhedron.palp_database... succeeded
    load sage.geometry.polyhedron.ppl_lattice_polygon... succeeded
**********************************************************************

comment:11 Changed 2 years ago by gh-kliem

This seems to be caused by #28605.

Just removing .cc to get rid of the empty folder doesn't seems to work.

comment:12 Changed 2 years ago by git

  • Commit changed from 846f2165017f94e9abb4fb30e4be05a8f4df439f to 655008373e17d4fe5dd7c337558c7ee61f3e8d41

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

202cad9replace attributes by methods
2e3e464removed empty folder being created in source
7701062removed attribute Vinv, as its not being used
1b17f6eadded docstrings to the new methods
c51cdd9removed method for Vinv
dd21f9capplied changes of 28605 to new code from 28625
4f49eacundid change to module list
6550083unbounded(self) -> is_bounded(self); _unbounded -> _bounded

comment:13 Changed 2 years ago by gh-kliem

  • Description modified (diff)
  • Status changed from needs_work to needs_review

Rebased it to #28605 so that there is only one commit now.

comment:14 Changed 2 years ago by gh-kliem

  • Status changed from needs_review to needs_work

Merge conflict

comment:15 Changed 2 years ago by gh-kliem

  • Branch changed from public/28606 to public/28606-reb
  • Commit changed from 655008373e17d4fe5dd7c337558c7ee61f3e8d41 to 8aaaef584b4da3d52376bdef769b4f13e192c542
  • Dependencies changed from #28625, #28605 to #28605
  • Status changed from needs_work to needs_review

New commits:

0f3b121replace attributes by methods; remove attribute Vinv
8aaaef5unbounded(self) -> is_bounded(self); _unbounded -> _bounded

comment:16 Changed 2 years ago by git

  • Commit changed from 8aaaef584b4da3d52376bdef769b4f13e192c542 to e44a5577c932f5ee7781f00112dd5237f8fdb41a

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

b0eac08replace attributes by methods; remove attribute Vinv
e44a557unbounded(self) -> is_bounded(self); _unbounded -> _bounded

comment:17 Changed 2 years ago by gh-kliem

  • Branch changed from public/28606-reb to public/28606-reb2
  • Commit changed from e44a5577c932f5ee7781f00112dd5237f8fdb41a to c7da39d30e85b8f0e464e4803e1230ad191afc45

rebased


New commits:

dd0729dreplace attributes by methods; remove attribute Vinv
d4b3163small fix in ridges
c7da39dunbounded(self) -> is_bounded(self); _unbounded -> _bounded

comment:18 Changed 2 years ago by jipilab

  • Reviewers changed from Laith Rastanawi to Laith Rastanawi, Jean-Philippe Labbé
  • Status changed from needs_review to positive_review

Morally good to go.

comment:19 Changed 2 years ago by vbraun

  • Branch changed from public/28606-reb2 to c7da39d30e85b8f0e464e4803e1230ad191afc45
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.