Opened 3 years ago
Closed 3 years ago
#24835 closed enhancement (fixed)
Change error message in construction of polyhedron object
Reported by:  jipilab  Owned by:  

Priority:  major  Milestone:  sage8.3 
Component:  geometry  Keywords:  days93, IMAPolyGeom 
Cc:  vdelecroix, moritz  Merged in:  
Authors:  JeanPhilippe Labbé  Reviewers:  Moritz Firsching 
Report Upstream:  N/A  Work issues:  
Branch:  7005c9e (Commits, GitHub, GitLab)  Commit:  7005c9e2175cc04058f08b7c3cad01379b64a2c1 
Dependencies:  #22572  Stopgaps: 
Description
The current error message given by
sage: Polyhedron([[sqrt(2)]]) Traceback (most recent call last): ... ValueError: for polyhedra with floating point numbers, the only allowed ring is RDF with backend 'cdd'
should be replaced by "ValueError?: the only allowed inexact ring is RDF with backend 'cdd'"
Change History (17)
comment:1 followups: ↓ 4 ↓ 5 Changed 3 years ago by
comment:2 Changed 3 years ago by
 Dependencies set to 22572
comment:3 Changed 3 years ago by
 Dependencies changed from 22572 to #22572
comment:4 in reply to: ↑ 1 Changed 3 years ago by
Replying to moritz:
Wouldn't it be good, if it automatically detected
AA
? (Perhaps not..)sage: P = Polyhedron([[sqrt(2)]], base_ring=AA) sage: P.vertices() (A vertex at (1.414213562373095?),)
I don't want to special case input coming from the symbolic ring in the polyhedron code. This is the kind of thing that would have to be repeated everywhere. So either sqrt(2)
should be a proper number field element (possibly AA) or this will stay as it is.
comment:5 in reply to: ↑ 1 Changed 3 years ago by
Replying to moritz:
Wouldn't it be good, if it automatically detected
AA
? (Perhaps not..)sage: P = Polyhedron([[sqrt(2)]], base_ring=AA) sage: P.vertices() (A vertex at (1.414213562373095?),)
Forcing the base ring to be algebraic numbers is a bit "DWIM"... so I would allow it as it is above. If the user is lazy and still puts the base ring to be AA
with square roots everywhere... it's still nice to be able to get what you want, but it should not allow to simply but symbolic stuff at all.
comment:6 Changed 3 years ago by
 Branch set to u/jipilab/24835
 Commit set to 84ec284c50cf936645f7befd6f50324e40e42e55
comment:7 Changed 3 years ago by
 Status changed from new to needs_review
comment:8 Changed 3 years ago by
 Commit changed from 84ec284c50cf936645f7befd6f50324e40e42e55 to 85dc6f4abcbc8a02c3a643a9f46455c1018ef006
Branch pushed to git repo; I updated commit sha1. New commits:
85dc6f4  pep8

comment:9 Changed 3 years ago by
 Commit changed from 85dc6f4abcbc8a02c3a643a9f46455c1018ef006 to 5e2b8e5b26389acef88cfd256afb3eac1da7869a
Branch pushed to git repo; I updated commit sha1. New commits:
5e2b8e5  Merge branch 'develop' into 24835

comment:10 Changed 3 years ago by
 Commit changed from 5e2b8e5b26389acef88cfd256afb3eac1da7869a to b32299ac40239dd5dd98abf05682123ec5aa22b0
Branch pushed to git repo; I updated commit sha1. Last 10 new commits:
31edde6  Completed a comment

231cd56  deleted trailing whitespace

f685c64  fixed some typos in lectures.rst

7f5e125  fixed typos in polyhedra_quickref.rst

bd6c356  LateX > LaTeX in polytope_tikz.rst

b1ce45b  Several other corrections

d526f70  renamed tutorial files

d7896f8  Merge branch 'develop' into 22572

597b802  Merge branch sage8.2.rc1 into 22572

b32299a  Merge branch '22572' into 24835

comment:11 Changed 3 years ago by
 Reviewers set to Moritz Firsching
 Status changed from needs_review to positive_review
Looks good!
comment:12 Changed 3 years ago by
 Commit changed from b32299ac40239dd5dd98abf05682123ec5aa22b0 to 7005c9e2175cc04058f08b7c3cad01379b64a2c1
 Status changed from positive_review to needs_review
comment:13 Changed 3 years ago by
Merge the dependency.
comment:14 Changed 3 years ago by
 Status changed from needs_review to positive_review
comment:15 Changed 3 years ago by
 Keywords IMAPolyGeom added
comment:16 Changed 3 years ago by
 Milestone changed from sage8.2 to sage8.3
comment:17 Changed 3 years ago by
 Branch changed from u/jipilab/24835 to 7005c9e2175cc04058f08b7c3cad01379b64a2c1
 Resolution set to fixed
 Status changed from positive_review to closed
Wouldn't it be good, if it automatically detected
AA
? (Perhaps not..)