Opened 2 years ago
Closed 2 years ago
#28614 closed enhancement (fixed)
CombinatorialPolyhedron: length_* to n_*
Reported by:  ghkliem  Owned by:  

Priority:  major  Milestone:  sage9.0 
Component:  geometry  Keywords:  polytopes, combinatorial polyhedron 
Cc:  jipilab, ghLaisRast  Merged in:  
Authors:  Jonathan Kliem  Reviewers:  Frédéric Chapoton 
Report Upstream:  N/A  Work issues:  
Branch:  2ad0ec0 (Commits, GitHub, GitLab)  Commit:  2ad0ec07c1dfb35b355e16e243f5c51ca5d9f988 
Dependencies:  Stopgaps: 
Description (last modified by )
To make CombinatorialPolyhedron
more consistent with Polyhedron
we change the following names.
length_Hrepr
>n_Hrepresentation
length_Vrepr
>n_Vrepresentation
In FaceIterator
:
length_atom_repr
>n_atom_rep
(note that #28608 changesrepr
torep
)
In CombinatorialFace
:
length_Vrepr
>n_ambient_Vrepresentation
length_Hrepr
>n_ambient_Hrepresentation
As both methods are public we keep the old methods with deprecation warnings.
As a follow up in #28615 we need to fix the alignment in src/sage/geometry/polyhedron/combinatorial_polyhedron/base.pxd
. Fixing it in this ticket would lead to merge conflicts and as its trivial, we can easily do it later.
Change History (20)
comment:1 Changed 2 years ago by
 Description modified (diff)
comment:2 Changed 2 years ago by
 Description modified (diff)
comment:3 Changed 2 years ago by
 Description modified (diff)
comment:4 Changed 2 years ago by
 Branch set to public/28614
 Commit set to 2bddb974048a483e869d03e5018efda5c70f5beb
comment:5 Changed 2 years ago by
 Commit changed from 2bddb974048a483e869d03e5018efda5c70f5beb to c03065126194dc15a8222a7ea32a6c8c800a40b4
Branch pushed to git repo; I updated commit sha1. New commits:
c030651  deprecation warnings; n_Vrepresentation > n_ambient_Vrepresentation in CombinatorialFace

comment:6 Changed 2 years ago by
 Cc jipilab ghLaisRast added
 Description modified (diff)
 Keywords polytopes combinatorial polyhedron added
 Status changed from new to needs_review
New commits:
c030651  deprecation warnings; n_Vrepresentation > n_ambient_Vrepresentation in CombinatorialFace

comment:7 Changed 2 years ago by
 Description modified (diff)
comment:8 Changed 2 years ago by
 Commit changed from c03065126194dc15a8222a7ea32a6c8c800a40b4 to c0c5262298b7b0b450ec7fe6bb21bbbe2ddca8aa
Branch pushed to git repo; I updated commit sha1. New commits:
c0c5262  altered the deprecation message to be the correct one for methods

comment:9 Changed 2 years ago by
 Dependencies changed from #28605 to #28605, #28606
Actually due to merge conflict, this ticket depends on #28606 as well.
comment:10 Changed 2 years ago by
 Commit changed from c0c5262298b7b0b450ec7fe6bb21bbbe2ddca8aa to e6c672ebbb77efb30ed9a42792c2ca68aa7463f2
Branch pushed to git repo; I updated commit sha1. Last 10 new commits:
dfbe2ad  Merge branch 'public/28607' of git://trac.sagemath.org/sage into public/28621

ed5518b  used CombinatorialPolyhedron to compute f_vector

9bdd005  give an error message for polytopes in some cases; removed incorrect example

acd671d  now we get a precice error message for inexact truncated dodecahedron

bf85a62  subsequent calls for f_vector fail if first attempt fails

dc99ea4  Merge branch 'public/28625' of git://trac.sagemath.org/sage into public/28605

9b5bcaa  applied changes of 28605 to new code from 28625

6fb97dc  Merge branch 'public/28605' of git://trac.sagemath.org/sage into public/28606

846f216  small fix in doc

e6c672e  merged in #28606; applied changes to new code from #28625

comment:11 Changed 2 years ago by
 Commit changed from e6c672ebbb77efb30ed9a42792c2ca68aa7463f2 to 6b376867af1cef2665143e86f3f4c23e8843ffdb
comment:12 Changed 2 years ago by
I rebased to #28606.
comment:14 Changed 2 years ago by
 Branch changed from public/28614 to public/28614reb
 Commit changed from 6b376867af1cef2665143e86f3f4c23e8843ffdb to 39cc0985af730981a707ae37afdf42e22473382b
 Dependencies #28605, #28606 deleted
comment:15 Changed 2 years ago by
 Status changed from needs_work to needs_review
comment:16 Changed 2 years ago by
 Commit changed from 39cc0985af730981a707ae37afdf42e22473382b to f14e75b4b2c236618e2b6bdfb825de5d0cf50614
Branch pushed to git repo; I updated commit sha1. New commits:
f14e75b  missed a replacement

comment:17 Changed 2 years ago by
do not import anything from __future__
in pyx files
otherwise, looks good
comment:18 Changed 2 years ago by
 Commit changed from f14e75b4b2c236618e2b6bdfb825de5d0cf50614 to 2ad0ec07c1dfb35b355e16e243f5c51ca5d9f988
Branch pushed to git repo; I updated commit sha1. New commits:
2ad0ec0  removed `from __future__` import in pyx files

comment:19 Changed 2 years ago by
 Reviewers set to Frédéric Chapoton
 Status changed from needs_review to positive_review
ok
comment:20 Changed 2 years ago by
 Branch changed from public/28614reb to 2ad0ec07c1dfb35b355e16e243f5c51ca5d9f988
 Resolution set to fixed
 Status changed from positive_review to closed
New commits:
replace attributes by methods
removed empty folder being created in source
replace attributes by methods; remove empty folder from source
removed attribute Vinv, as its not being used
added docstrings to the new methods
removed method for Vinv
length_* > n_