Opened 7 years ago
Closed 7 years ago
#19954 closed enhancement (fixed)
QQbar cleaning 1
Reported by:  Vincent Delecroix  Owned by:  

Priority:  major  Milestone:  sage7.1 
Component:  number fields  Keywords:  
Cc:  Merged in:  
Authors:  Vincent Delecroix  Reviewers:  Frédéric Chapoton 
Report Upstream:  N/A  Work issues:  
Branch:  c1f5c5f (Commits, GitHub, GitLab)  Commit:  c1f5c5fb96bf5bb06c4dacdaf132d7ce2b3c8de3 
Dependencies:  Stopgaps: 
Description (last modified by )
This is a series of ticket about QQbar
cleaning. In this first one we:
 get rid of
ANRootOfUnity
descriptor  get rid of special casing for 'zero', 'rootunity', 'imaginary'
 move the 34gon commented function as a doctest
By doing so we slow down the code for minpoly
but this will be fixed in the subsequent ticket #19955.
This ticket is part of the task #18333. See also the follow up #20074.
Change History (18)
comment:1 Changed 7 years ago by
Description:  modified (diff) 

comment:2 Changed 7 years ago by
Branch:  → u/vdelecroix/19954 

Commit:  → 7ab3ffbfd3e824c73a1c78d59de84b37f4b94e2c 
Status:  new → needs_review 
comment:3 Changed 7 years ago by
Status:  needs_review → needs_work 

failing doctests, see patchbot report
comment:4 Changed 7 years ago by
Commit:  7ab3ffbfd3e824c73a1c78d59de84b37f4b94e2c → d1788a00924a77f218bbff8c74d0eb56a7afa55c 

comment:5 Changed 7 years ago by
Status:  needs_work → needs_review 

comment:7 Changed 7 years ago by
Commit:  d1788a00924a77f218bbff8c74d0eb56a7afa55c → 94629dbef9c590f0f748d1c0e40e5833f226c245 

Branch pushed to git repo; I updated commit sha1. New commits:
94629db  Trac 19954: doctest independent of execution order

comment:8 Changed 7 years ago by
I see. The doctest was dependent of execution order. I changed for a more neutral one.
comment:9 Changed 7 years ago by
I have found a typo:
Class to upport old unpickling
And I am a bit puzzled by the very complicated shape of the new results of the sage_input
method.
comment:10 Changed 7 years ago by
The sage_input
is just intended to reconstruct the object. It is the very same with
sage: A = identity_matrix(4) sage: sage_input(A) matrix(ZZ, [[1, 0, 0, 0], [0, 1, 0, 0], [0, 0, 1, 0], [0, 0, 0, 1]])
the answer is hopefully not identity_matrix(4)
!
Before this ticket, the algebraic numbers used to remember the way they were constructed. Which is a complete nonsense.
comment:11 Changed 7 years ago by
Commit:  94629dbef9c590f0f748d1c0e40e5833f226c245 → cb2592cf73f815ccb611babbd18a3be19e95b235 

Branch pushed to git repo; I updated commit sha1. New commits:
cb2592c  Trac 19954: typo in documentation

comment:12 Changed 7 years ago by
the description at the top of the documentation still refers to
Algebraic numbers exist in one of the following forms: a rational number the product of a rational number and an n‘th root of unity etc
The second line should be removed, no ?
comment:13 Changed 7 years ago by
Commit:  cb2592cf73f815ccb611babbd18a3be19e95b235 → c1f5c5fb96bf5bb06c4dacdaf132d7ce2b3c8de3 

comment:14 Changed 7 years ago by
Right! I also remove functions relative to "gaussian" which no longer exists.
comment:15 Changed 7 years ago by
Description:  modified (diff) 

comment:16 Changed 7 years ago by
Reviewers:  → Frédéric Chapoton 

Status:  needs_review → positive_review 
ok, this seems to be good to go.
One can note that there is the UniversalCyclotomicField
for people only caring
about the abelian closure of QQ
.
comment:18 Changed 7 years ago by
Branch:  u/vdelecroix/19954 → c1f5c5fb96bf5bb06c4dacdaf132d7ce2b3c8de3 

Resolution:  → fixed 
Status:  positive_review → closed 
New commits:
Trac 19954: get rid of ANRootOfUnity
Trac 19954: get rid of the argument _is_pow for ANRoot
Trac 19954: fix doctests
Trac 19954: fix sage_input doctests
Trac 19954: move the 34gon as a doctest