Opened 7 years ago
Closed 6 years ago
#16627 closed defect (fixed)
Slight inconsistency in base ring of polytope doc
Reported by:  kcrisman  Owned by:  

Priority:  trivial  Milestone:  sage6.8 
Component:  geometry  Keywords:  beginner 
Cc:  Merged in:  
Authors:  Moritz Firsching  Reviewers:  JeanPhilippe Labbé 
Report Upstream:  N/A  Work issues:  
Branch:  fbc0d3d (Commits, GitHub, GitLab)  Commit:  fbc0d3d12a77f1d619c7a981f00001c41b450082 
Dependencies:  Stopgaps: 
Description
sage: P8 = polytopes.n_cube(4) sage: P8.base_ring? Type: instancemethod String form: <bound method Polyhedra_ZZ_ppl_with_category.element_class.base_ring of A 4dimensional polyhedron in ZZ^4 defined as the convex hull of 16 vertices> Definition: P8.base_ring(self) Docstring: Return the base ring. OUTPUT: Either "QQ" (exact arithmetic using gmp, default) or "RDF" (double precision floatingpoint arithmetic) EXAMPLES: sage: triangle = Polyhedron(vertices = [[1,0],[0,1],[1,1]]) sage: triangle.base_ring() == ZZ True
I particularly like how the example directly contradicts the documentation ;) Anyway, all options should be tested.
Change History (11)
comment:1 Changed 7 years ago by
 Milestone changed from sage6.3 to sage6.4
comment:2 Changed 7 years ago by
 Branch set to u/moritz/ticket/16627
 Commit set to e49a4c7bf169f37133cd7c5118ec225406318ed4
 Status changed from new to needs_review
comment:3 Changed 7 years ago by
Looks like that is all the options:
raise TypeError('The base ring must be ZZ, QQ, or RDF')
Although this was supposed to be a very simple ticket, I wonder if I may as reviewer ask for two doctests testing the cdd reps get mad at bad base ring input...
comment:4 Changed 7 years ago by
 Branch changed from u/moritz/ticket/16627 to u/vbraun/ticket/16627
comment:5 Changed 7 years ago by
 Commit changed from e49a4c7bf169f37133cd7c5118ec225406318ed4 to a8667eb3725f0156cc3936dd770e8a3e04852cf1
We now have support for arbitrary rings, updated accordingly.
New commits:
a8667eb  Fix base_ring docstring

comment:6 Changed 7 years ago by
So if the ring isn't ZZ,QQ,RDF
then the error code in the cdd h/v representations about that (see above) is never reached, or is no longer correct? Just checking.
comment:7 Changed 7 years ago by
The errors are only raised in the CDD format output which only supports these formats.
comment:8 Changed 7 years ago by
 Branch changed from u/vbraun/ticket/16627 to u/moritz/ticket/16627v2
 Commit changed from a8667eb3725f0156cc3936dd770e8a3e04852cf1 to fbc0d3d12a77f1d619c7a981f00001c41b450082
I added a doctest for the cdd format
comment:9 Changed 6 years ago by
 Milestone changed from sage6.4 to sage6.8
This patch looks good to me. If the latest bot tests like it too I would set it to positive review.
comment:10 Changed 6 years ago by
 Reviewers set to JeanPhilippe Labbé
 Status changed from needs_review to positive_review
comment:11 Changed 6 years ago by
 Branch changed from u/moritz/ticket/16627v2 to fbc0d3d12a77f1d619c7a981f00001c41b450082
 Resolution set to fixed
 Status changed from positive_review to closed
I included "ZZ" in the doc.