search

sagemath / sage

Main repository of SageMath
https://www.sagemath.org
Other
1.36k stars 462 forks source link

CombinatorialPolyhedron: length_* to n_* #28614

Closed kliem closed 4 years ago

kliem commented 4 years ago

To make CombinatorialPolyhedron more consistent with Polyhedron we change the following names.

In FaceIterator:

In CombinatorialFace:

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.

CC: @jplab @LaisRast

Component: geometry

Keywords: polytopes, combinatorial polyhedron

Author: Jonathan Kliem

Branch/Commit: 2ad0ec0

Reviewer: Frédéric Chapoton

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

kliem commented 4 years ago

Description changed:

--- 
+++ 
@@ -8,3 +8,5 @@

 In `CombinatorialFace`:
 `length_Vrepresentation` -> `n_ambient_Vrepresentation`
+
+As a follow up 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.
kliem commented 4 years ago

Description changed:

--- 
+++ 
@@ -1,12 +1,12 @@
 To make `CombinatorialPolyhedron` more consistent with `Polyhedron` we change the following names.

-`length_Hrepr` -> `n_Hrepresentation`
-`length_Vrepr` -> `n_Vrepresentation`
+- `length_Hrepr` -> `n_Hrepresentation`
+- `length_Vrepr` -> `n_Vrepresentation`

 In `FaceIterator`:
-`length_atom_repr` -> `n_atom_rep` (note that #28608 changes `repr` to `rep`)
+- `length_atom_repr` -> `n_atom_rep` (note that #28608 changes `repr` to `rep`)

 In `CombinatorialFace`:
-`length_Vrepresentation` -> `n_ambient_Vrepresentation`
+- `length_Vrepresentation` -> `n_ambient_Vrepresentation`

 As a follow up 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.
kliem commented 4 years ago

Description changed:

--- 
+++ 
@@ -7,6 +7,6 @@
 - `length_atom_repr` -> `n_atom_rep` (note that #28608 changes `repr` to `rep`)

 In `CombinatorialFace`:
-- `length_Vrepresentation` -> `n_ambient_Vrepresentation`
+- `length_Vrepr` -> `n_ambient_Vrepresentation`

 As a follow up 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.
kliem commented 4 years ago

Commit: 2bddb97

kliem commented 4 years ago

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
2bddb97length_* -> n_
kliem commented 4 years ago

Branch: public/28614

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 4 years ago

Changed commit from 2bddb97 to c030651

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 4 years ago

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

c030651deprecation warnings; n_Vrepresentation -> n_ambient_Vrepresentation in CombinatorialFace
kliem commented 4 years ago

New commits:

c030651deprecation warnings; n_Vrepresentation -> n_ambient_Vrepresentation in CombinatorialFace
kliem commented 4 years ago

Changed keywords from none to polytopes, combinatorial polyhedron

kliem commented 4 years ago

Description changed:

--- 
+++ 
@@ -8,5 +8,7 @@

 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 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.
kliem commented 4 years ago

Description changed:

--- 
+++ 
@@ -11,4 +11,4 @@
 - `length_Hrepr` -> `n_ambient_Hrepresentation`
 As both methods are public we keep the old methods with deprecation warnings.

-As a follow up 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.
+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.
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 4 years ago

Changed commit from c030651 to c0c5262

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 4 years ago

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

c0c5262altered the deprecation message to be the correct one for methods
kliem commented 4 years ago
comment:9

Actually due to merge conflict, this ticket depends on #28606 as well.

kliem commented 4 years ago

Changed dependencies from #28605 to #28605, #28606

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 4 years ago

Changed commit from c0c5262 to e6c672e

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 4 years ago

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

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
e6c672emerged in #28606; applied changes to new code from #28625
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 4 years ago

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

2c039cclength_* -> n_
a22010cdeprecation warnings; n_Vrepresentation -> n_ambient_Vrepresentation in CombinatorialFace
6b37686altered the deprecation message to be the correct one for methods
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 4 years ago

Changed commit from e6c672e to 6b37686

kliem commented 4 years ago
comment:12

I rebased to #28606.

kliem commented 4 years ago
comment:13

Waiting on #28606.

kliem commented 4 years ago

Changed commit from 6b37686 to 39cc098

kliem commented 4 years ago

Changed dependencies from #28605, #28606 to none

kliem commented 4 years ago

New commits:

7acef4clength_* -> n_
ade3dcadeprecation warnings; n_Vrepresentation -> n_ambient_Vrepresentation in CombinatorialFace
39cc098altered the deprecation message to be the correct one for methods
kliem commented 4 years ago

Changed branch from public/28614 to public/28614-reb

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 4 years ago

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

f14e75bmissed a replacement
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 4 years ago

Changed commit from 39cc098 to f14e75b

fchapoton commented 4 years ago
comment:17

do not import anything from __future__ in pyx files

otherwise, looks good

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 4 years ago

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

2ad0ec0removed `from __future__` import in pyx files
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 4 years ago

Changed commit from f14e75b to 2ad0ec0

fchapoton commented 4 years ago

Reviewer: Frédéric Chapoton

fchapoton commented 4 years ago
comment:19

ok

vbraun commented 4 years ago

Changed branch from public/28614-reb to 2ad0ec0