sagemath/sage

Meta-ticket: Split sage.geometry.polyhedron.base

Closed this issue · 15 comments

mkoeppe commented

This is a pretty large module, consisting of 11 kLOC.

We could split it into several modules, each defining an intermediate class (like it is done in sage.matrix.matrix0, matrix1, matrix2).

For example,

  • a module sage.geometry.polyhedron.base0 contains the very basic methods (initialization etc) #32767.
  • a module sage.geometry.polyhedron.base1 could contain the implementation of the methods defined by the ConvexSet_base API, #32776
  • a module sage.geometry.polyhedron.base2 could contain all methods that have to do with lattice points in polyhedra, #32876
  • a module sage.geometry.polyhedron.base3 contains the combinatorial methods, #32884
  • a module sage.geometry.polyhedron.base4 could contain all methods whose implementation needs sage.graphs.graphs, #32889 -- this would help with modularization because then we could conditionalize the doctests via a file-level directive (#30778)
    • vertex_graph, graph, vertex_digraph, vertex_facet_graph
    • hasse_diagram
    • face_lattice, flag_f_vector
    • {combinatorial,restricted}_automorphism_group
    • is_combinatorially_isomorphic
  • a module sage.geometry.polyhedron.base5 could contain all constructions of new polyhedra, #33123
  • a module sage.geometry.polyhedron.base6 could contain all methods related to plotting and affine hull (projection), #33449
  • a module sage.geometry.polyhedron.base7 could contain all methods related to triangulation and volume, #33583

This could also lead to an improvement of the documentation.

CC: @kliem @jplab @tscrim

Component: refactoring

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

mkoeppe commented

Description changed:

--- 
+++ 
@@ -7,4 +7,5 @@
 - a module `sage.geometry.polyhedron.base_lattice` could contain all methods that have to do with lattice points in polyhedra
 - a module `sage.geometry.polyhedron.base_graphs` could contain all methods whose implementation needs `sage.graphs.graphs` -- this would help with modularization because then we could conditionalize the doctests via a file-level directive (#30778)
 
+This could also lead to an improvement of the documentation.
kliem commented

Description changed:

--- 
+++ 
@@ -3,6 +3,7 @@
 We could split it into several modules, each defining an intermediate class (like it is done in `sage.matrix.matrix0, matrix1, matrix2`).
 
 For example,
+- a module `sage.geometry.polyhedron.base0` contains the very basic methods (initialization etc) #32767.
 - a module `sage.geometry.polyhedron.base_convex` could contain the implementation of the methods defined by the `ConvexSet_base` API
 - a module `sage.geometry.polyhedron.base_lattice` could contain all methods that have to do with lattice points in polyhedra
 - a module `sage.geometry.polyhedron.base_graphs` could contain all methods whose implementation needs `sage.graphs.graphs` -- this would help with modularization because then we could conditionalize the doctests via a file-level directive (#30778)
kliem commented

Description changed:

--- 
+++ 
@@ -4,7 +4,7 @@
 
 For example,
 - a module `sage.geometry.polyhedron.base0` contains the very basic methods (initialization etc) #32767.
-- a module `sage.geometry.polyhedron.base_convex` could contain the implementation of the methods defined by the `ConvexSet_base` API
+- a module `sage.geometry.polyhedron.base_convex` could contain the implementation of the methods defined by the `ConvexSet_base` API,  #32776
 - a module `sage.geometry.polyhedron.base_lattice` could contain all methods that have to do with lattice points in polyhedra
 - a module `sage.geometry.polyhedron.base_graphs` could contain all methods whose implementation needs `sage.graphs.graphs` -- this would help with modularization because then we could conditionalize the doctests via a file-level directive (#30778)
mkoeppe commented

Description changed:

--- 
+++ 
@@ -4,9 +4,15 @@
 
 For example,
 - a module `sage.geometry.polyhedron.base0` contains the very basic methods (initialization etc) #32767.
-- a module `sage.geometry.polyhedron.base_convex` could contain the implementation of the methods defined by the `ConvexSet_base` API,  #32776
+- a module `sage.geometry.polyhedron.base1` could contain the implementation of the methods defined by the `ConvexSet_base` API,  #32776
 - a module `sage.geometry.polyhedron.base_lattice` could contain all methods that have to do with lattice points in polyhedra
 - a module `sage.geometry.polyhedron.base_graphs` could contain all methods whose implementation needs `sage.graphs.graphs` -- this would help with modularization because then we could conditionalize the doctests via a file-level directive (#30778)
+  - `vertex_graph`, `graph`, `vertex_digraph`, `vertex_facet_graph`
+  - `hasse_diagram`
+  - `face_lattice`, `flag_f_vector`
+  - `{combinatorial,restricted}_automorphism_group`
+  - `is_combinatorially_isomorphic`
+  
 
 This could also lead to an improvement of the documentation.
kliem commented

Description changed:

--- 
+++ 
@@ -5,7 +5,7 @@
 For example,
 - a module `sage.geometry.polyhedron.base0` contains the very basic methods (initialization etc) #32767.
 - a module `sage.geometry.polyhedron.base1` could contain the implementation of the methods defined by the `ConvexSet_base` API,  #32776
-- a module `sage.geometry.polyhedron.base_lattice` could contain all methods that have to do with lattice points in polyhedra
+- a module `sage.geometry.polyhedron.base2` could contain all methods that have to do with lattice points in polyhedra, #32876
 - a module `sage.geometry.polyhedron.base_graphs` could contain all methods whose implementation needs `sage.graphs.graphs` -- this would help with modularization because then we could conditionalize the doctests via a file-level directive (#30778)
   - `vertex_graph`, `graph`, `vertex_digraph`, `vertex_facet_graph`
   - `hasse_diagram`
kliem commented

Description changed:

--- 
+++ 
@@ -6,6 +6,7 @@
 - a module `sage.geometry.polyhedron.base0` contains the very basic methods (initialization etc) #32767.
 - a module `sage.geometry.polyhedron.base1` could contain the implementation of the methods defined by the `ConvexSet_base` API,  #32776
 - a module `sage.geometry.polyhedron.base2` could contain all methods that have to do with lattice points in polyhedra, #32876
+- a module `sage.geometry.polyhedron.base2` contains the combinatorial methods, #32884
 - a module `sage.geometry.polyhedron.base_graphs` could contain all methods whose implementation needs `sage.graphs.graphs` -- this would help with modularization because then we could conditionalize the doctests via a file-level directive (#30778)
   - `vertex_graph`, `graph`, `vertex_digraph`, `vertex_facet_graph`
   - `hasse_diagram`
slel commented

Description changed:

--- 
+++ 
@@ -6,7 +6,7 @@
 - a module `sage.geometry.polyhedron.base0` contains the very basic methods (initialization etc) #32767.
 - a module `sage.geometry.polyhedron.base1` could contain the implementation of the methods defined by the `ConvexSet_base` API,  #32776
 - a module `sage.geometry.polyhedron.base2` could contain all methods that have to do with lattice points in polyhedra, #32876
-- a module `sage.geometry.polyhedron.base2` contains the combinatorial methods, #32884
+- a module `sage.geometry.polyhedron.base3` contains the combinatorial methods, #32884
 - a module `sage.geometry.polyhedron.base_graphs` could contain all methods whose implementation needs `sage.graphs.graphs` -- this would help with modularization because then we could conditionalize the doctests via a file-level directive (#30778)
   - `vertex_graph`, `graph`, `vertex_digraph`, `vertex_facet_graph`
   - `hasse_diagram`
kliem commented

Description changed:

--- 
+++ 
@@ -7,13 +7,15 @@
 - a module `sage.geometry.polyhedron.base1` could contain the implementation of the methods defined by the `ConvexSet_base` API,  #32776
 - a module `sage.geometry.polyhedron.base2` could contain all methods that have to do with lattice points in polyhedra, #32876
 - a module `sage.geometry.polyhedron.base3` contains the combinatorial methods, #32884
-- a module `sage.geometry.polyhedron.base_graphs` could contain all methods whose implementation needs `sage.graphs.graphs` -- this would help with modularization because then we could conditionalize the doctests via a file-level directive (#30778)
+- a module `sage.geometry.polyhedron.base4` could contain all methods whose implementation needs `sage.graphs.graphs`, #32889 -- this would help with modularization because then we could conditionalize the doctests via a file-level directive (#30778)
   - `vertex_graph`, `graph`, `vertex_digraph`, `vertex_facet_graph`
   - `hasse_diagram`
   - `face_lattice`, `flag_f_vector`
   - `{combinatorial,restricted}_automorphism_group`
   - `is_combinatorially_isomorphic`
-  
+-  a module `sage.geometry.polyhedron.base_constructions` could contain all methods all constructions of new polyhedra
+- a module `sage.geometry.polyhedron.base_plot` could contain all methods related to plotting
+- a module `sage.geometry.polyhedron.base_triangulate` could contain all methods related to triangulation and volume
 
 This could also lead to an improvement of the documentation.
kliem commented

Description changed:

--- 
+++ 
@@ -13,7 +13,7 @@
   - `face_lattice`, `flag_f_vector`
   - `{combinatorial,restricted}_automorphism_group`
   - `is_combinatorially_isomorphic`
--  a module `sage.geometry.polyhedron.base_constructions` could contain all methods all constructions of new polyhedra
+-  a module `sage.geometry.polyhedron.base5` could contain all constructions of new polyhedra, #33123
 - a module `sage.geometry.polyhedron.base_plot` could contain all methods related to plotting
 - a module `sage.geometry.polyhedron.base_triangulate` could contain all methods related to triangulation and volume
kliem commented

Description changed:

--- 
+++ 
@@ -14,7 +14,7 @@
   - `{combinatorial,restricted}_automorphism_group`
   - `is_combinatorially_isomorphic`
 -  a module `sage.geometry.polyhedron.base5` could contain all constructions of new polyhedra, #33123
-- a module `sage.geometry.polyhedron.base_plot` could contain all methods related to plotting
+- a module `sage.geometry.polyhedron.base6` could contain all methods related to plotting and affine hull (projection), #33449
 - a module `sage.geometry.polyhedron.base_triangulate` could contain all methods related to triangulation and volume
 
 This could also lead to an improvement of the documentation.
kliem commented

Description changed:

--- 
+++ 
@@ -15,7 +15,7 @@
   - `is_combinatorially_isomorphic`
 -  a module `sage.geometry.polyhedron.base5` could contain all constructions of new polyhedra, #33123
 - a module `sage.geometry.polyhedron.base6` could contain all methods related to plotting and affine hull (projection), #33449
-- a module `sage.geometry.polyhedron.base_triangulate` could contain all methods related to triangulation and volume
+- a module `sage.geometry.polyhedron.base7` could contain all methods related to triangulation and volume, #33583
 
 This could also lead to an improvement of the documentation.
kliem commented
comment:13

Is this done with #33583? What did you have in mind with "This could also lead to an improvement of the documentation."?

jplab commented
comment:14

That's a great ticket! ... I'm not dead. I expect to be "back on trac" soon!

I would be up for improving the doc and complement it further... I'll slowly go through all this new stuff in polyhedron this Summer.

Thanks for all the work!

jhpalmieri commented
comment:17

Is this done?

mkoeppe commented
comment:18

It's done except we didn't get around to do "an improvement of the documentation" (comment:13) -- in the course of which we would surely have noticed that the documentation for all of this is no longer being built (#25675).