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: | sage-8.3 |
Component: | geometry | Keywords: | days93, IMA-PolyGeom |
Cc: | vdelecroix, moritz | Merged in: | |
Authors: | Jean-Philippe 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 follow-ups: ↓ 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 IMA-PolyGeom added
comment:16 Changed 3 years ago by
- Milestone changed from sage-8.2 to sage-8.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..)