Opened 2 years ago
Closed 2 years ago
#23948 closed enhancement (fixed)
has_cyclotomic_factor for polynomials
Reported by:  kedlaya  Owned by:  

Priority:  minor  Milestone:  sage8.1 
Component:  algebra  Keywords:  sd91 
Cc:  Merged in:  
Authors:  Kiran Kedlaya  Reviewers:  Simon Brandhorst, David Roe 
Report Upstream:  N/A  Work issues:  
Branch:  14595cb (Commits)  Commit:  14595cb384f26e422ee0aeeba2170ecfca42025a 
Dependencies:  Stopgaps: 
Description
This ticket adds a method has_cyclotomic_factor
for polynomials, which returns True if and only if the polynomial has a nontrivial factor which is a cyclotomic polynomial (i.e., if and only if at least one root of the polynomial is a root of unity).
This is an offshoot of #23946.
Change History (23)
comment:1 Changed 2 years ago by
 Branch set to u/kedlaya/t/kedlaya/has_cyclotomic_factor
comment:2 Changed 2 years ago by
 Commit set to 4cdc320de7d09e4f72c95910df97d6bd82c3f701
comment:3 Changed 2 years ago by
 Commit changed from 4cdc320de7d09e4f72c95910df97d6bd82c3f701 to 82f5d38ef872e17b40f3587697369dc4775cfaa9
Branch pushed to git repo; I updated commit sha1. New commits:
82f5d38  Remove irreducible case (redundant)

comment:4 Changed 2 years ago by
 Commit changed from 82f5d38ef872e17b40f3587697369dc4775cfaa9 to 66293fa1e413076cd8f882fa44d0508e31752cfc
comment:5 Changed 2 years ago by
 Commit changed from 66293fa1e413076cd8f882fa44d0508e31752cfc to dc1c7f1d37f6c3a7e5c2fc315220d2b3e61d9ddf
Branch pushed to git repo; I updated commit sha1. New commits:
dc1c7f1  More comments in code

comment:6 Changed 2 years ago by
 Status changed from new to needs_review
comment:7 Changed 2 years ago by
 Reviewers set to Simon Brandhorst
comment:8 Changed 2 years ago by
 Commit changed from dc1c7f1d37f6c3a7e5c2fc315220d2b3e61d9ddf to c164825109557ad45f71421abb35b952a25dbd00
Branch pushed to git repo; I updated commit sha1. New commits:
c164825  Added random doctest

comment:9 Changed 2 years ago by
comment:10 Changed 2 years ago by
 Status changed from needs_review to positive_review
comment:11 Changed 2 years ago by
comment:12 Changed 2 years ago by
For the record, all tests passed on k8s. (There was a failure in src/sage/repl/configuration.py
which was unreproducible, so I'm treating it as spurious.)
comment:14 Changed 2 years ago by
 Commit changed from c164825109557ad45f71421abb35b952a25dbd00 to b8f9a3db6b7c3d3be4748eae5a9f0db21c4e0991
Branch pushed to git repo; I updated commit sha1. New commits:
b8f9a3d  Merge branch 'develop' of git://trac.sagemath.org/sage into t/23948/has_cyclotomic_factor

comment:15 Changed 2 years ago by
 Status changed from needs_work to needs_review
Weird, I didn't get a merge conflict just now. Anyway, see if this helps.
comment:16 Changed 2 years ago by
 Reviewers changed from Simon Brandhorst to Simon Brandhorst, David Roe
 Status changed from needs_review to positive_review
Looks good to me.
comment:18 Changed 2 years ago by
Sorry, I haven't been able to reproduce the merge conflict. Would it be possible to provide more details?
comment:19 Changed 2 years ago by
Presumably it's with the next beta, so we'll need to wait until that comes out.
comment:20 Changed 2 years ago by
 Branch changed from u/kedlaya/t/kedlaya/has_cyclotomic_factor to u/roed/t/kedlaya/has_cyclotomic_factor
comment:21 Changed 2 years ago by
 Commit changed from b8f9a3db6b7c3d3be4748eae5a9f0db21c4e0991 to 14595cb384f26e422ee0aeeba2170ecfca42025a
Branch pushed to git repo; I updated commit sha1. New commits:
14595cb  Fix mistaken removal of O function in laurent_series_ring_element

comment:22 Changed 2 years ago by
 Status changed from needs_work to positive_review
Fixed merge conflict
comment:23 Changed 2 years ago by
 Branch changed from u/roed/t/kedlaya/has_cyclotomic_factor to 14595cb384f26e422ee0aeeba2170ecfca42025a
 Resolution set to fixed
 Status changed from positive_review to closed
Branch pushed to git repo; I updated commit sha1. New commits:
Remove unrelated patch for Laurent series
Fix docstring formatting