Split initialization and basic access of polyhedra out as a separate module

Question

Split initialization and basic access of polyhedra out as a separate module

Closed this issue 4 years ago · 17 comments

Part of #32651.

Outsource initialization and very basic access (Vrepresentation, Hrepresentation, backend, base_ring) to a base class Polyhedron_base0.

Depends on #32637

CC: @kliem @jplab @tscrim @mkoeppe

Component: geometry

Author: Jonathan Kliem

Branch/Commit: 078cc56

Reviewer: Matthias Koeppe

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

Answer 1 · 2021-10-25T19:04:39.000Z

comment:2

Should dim, ambient_dim rather be in base_convex?

Answer 2 · 2021-10-25T19:34:15.000Z

comment:3

I think your suggestion makes more sense. They are not needed there yet, except for __repr__, which should also move up, as it isn't complete then.

Answer 3 · 2021-10-25T19:38:00.000Z

Changed commit from 9a48bd9 to 6dfadb8

Answer 4 · 2021-10-25T19:38:00.000Z

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

`6dfadb8`	`leave dim and ambient_dim for convex set`

Answer 5 · 2021-10-25T19:39:31.000Z

comment:5

Regarding the names, I don't know what makes most sense. I was thinking base0.py etc., as it is clear then, what the order is. However, we are not doing cython, so we don't need to be strict with inheritance and could use names also and try to have it more or less consistent.

Answer 6 · 2021-10-25T23:01:25.000Z

comment:6

The random failure

sage -t --long --random-seed=77478494819088915365500074763386376542 src/sage/rings/continued_fraction.py
**********************************************************************
File "src/sage/rings/continued_fraction.py", line 265, in sage.rings.continued_fraction.rat_interval_cf_list

is probably not from this ticket.

Answer 7 · 2021-10-25T23:01:43.000Z

comment:7

base0 is fine, I think

Answer 8 · 2021-10-25T23:12:34.000Z

comment:9

pyflakes reports:

src/sage/geometry/polyhedron/base.py:42:1 'sage.misc.superseded.deprecated_function_alias' imported but unused

Answer 9 · 2021-10-25T23:13:39.000Z

Dependencies: #32637

Answer 10 · 2021-10-25T23:18:07.000Z

Changed branch from u/gh-kliem/polyhedron_base0 to u/mkoeppe/polyhedron_base0

Answer 11 · 2021-10-25T23:18:27.000Z

comment:12

Merged #32637 to avoid merge conflict.

Last 10 new commits:

`eeaa722`	`Merge #32614`
`c872d69`	`Add description to features`
`08248a1`	`Fix for doctest failures`
`759c88b`	`sage.doctest, sage.control: Remove unused imports`
`15729dc`	`src/sage/features/mcqd.py: Add doctests`
`1d02bd0`	`More doctests for coverage`
`58572e1`	`Merge #32649`
`44540f5`	`src/sage/features/sagemath.py: Add doctests`
`a34658e`	`Merge #32637`
`301a5e0`	`src/sage/geometry/polyhedron/base.py: Remove unused import`

Answer 12 · 2021-10-25T23:18:27.000Z

Changed commit from 6dfadb8 to 301a5e0

Answer 13 · 2021-10-25T23:35:07.000Z

Changed commit from 301a5e0 to 078cc56

Answer 14 · 2021-10-25T23:35:07.000Z

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

`b8c3ac7`	`Merge #32637`
`078cc56`	`src/sage/geometry/polyhedron/base.py: Remove unused import`

Answer 15 · 2021-10-25T23:36:55.000Z

Reviewer: Matthias Koeppe

Answer 16 · 2021-10-26T08:38:04.000Z

comment:15

Thank you.

Answer 17 · 2021-10-31T22:20:44.000Z

Changed branch from u/mkoeppe/polyhedron_base0 to 078cc56