Opened 6 years ago
Closed 6 years ago
#19954 closed enhancement (fixed)
QQbar cleaning 1
Reported by:  vdelecroix  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 6 years ago by
 Description modified (diff)
comment:2 Changed 6 years ago by
 Branch set to u/vdelecroix/19954
 Commit set to 7ab3ffbfd3e824c73a1c78d59de84b37f4b94e2c
 Status changed from new to needs_review
comment:3 Changed 6 years ago by
 Status changed from needs_review to needs_work
failing doctests, see patchbot report
comment:4 Changed 6 years ago by
 Commit changed from 7ab3ffbfd3e824c73a1c78d59de84b37f4b94e2c to d1788a00924a77f218bbff8c74d0eb56a7afa55c
comment:5 Changed 6 years ago by
 Status changed from needs_work to needs_review
comment:6 Changed 6 years ago by
Still one doctest failing
comment:7 Changed 6 years ago by
 Commit changed from d1788a00924a77f218bbff8c74d0eb56a7afa55c to 94629dbef9c590f0f748d1c0e40e5833f226c245
Branch pushed to git repo; I updated commit sha1. New commits:
94629db  Trac 19954: doctest independent of execution order

comment:8 Changed 6 years ago by
I see. The doctest was dependent of execution order. I changed for a more neutral one.
comment:9 Changed 6 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 6 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 6 years ago by
 Commit changed from 94629dbef9c590f0f748d1c0e40e5833f226c245 to cb2592cf73f815ccb611babbd18a3be19e95b235
Branch pushed to git repo; I updated commit sha1. New commits:
cb2592c  Trac 19954: typo in documentation

comment:12 Changed 6 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 6 years ago by
 Commit changed from cb2592cf73f815ccb611babbd18a3be19e95b235 to c1f5c5fb96bf5bb06c4dacdaf132d7ce2b3c8de3
comment:14 Changed 6 years ago by
Right! I also remove functions relative to "gaussian" which no longer exists.
comment:15 Changed 6 years ago by
 Description modified (diff)
comment:16 Changed 6 years ago by
 Reviewers set to Frédéric Chapoton
 Status changed from needs_review to 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:17 Changed 6 years ago by
Thanks Frédéric!
comment:18 Changed 6 years ago by
 Branch changed from u/vdelecroix/19954 to c1f5c5fb96bf5bb06c4dacdaf132d7ce2b3c8de3
 Resolution set to fixed
 Status changed from positive_review to 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