Opened 5 years ago
Closed 5 years ago
#21194 closed defect (fixed)
Multiple bugs in Polynomial.reverse(degree)
Reported by:  mmezzarobba  Owned by:  

Priority:  major  Milestone:  sage7.4 
Component:  commutative algebra  Keywords:  
Cc:  Merged in:  
Authors:  Marc Mezzarobba  Reviewers:  Vincent Delecroix 
Report Upstream:  N/A  Work issues:  
Branch:  554500b (Commits, GitHub, GitLab)  Commit:  554500b7cecafee1c8596bc4a5c0300a5e82a89b 
Dependencies:  Stopgaps: 
Description (last modified by )
Polynomial.reverse(d) over ℚ is inconsistent with the generic implementation. The name of the optional argument is different, and its interpretation is slightly different:
sage: x = polygen(QQ) sage: y = polygen(QQbar) sage: (x+1).reverse(1) 1 sage: (y+1).reverse(1) x + 1
In addition, the documentation of the generic reverse()
(which arguably should specify what reverse()
is supposed to do for sage polynomials) incorrectly states that “the reverse polynomial will have the specified degree”:
sage: (y^2).reverse(5) x^3
Finally, the generic implementation is buggy when the optional argument is zero.
Change History (10)
comment:1 Changed 5 years ago by
 Description modified (diff)
comment:2 Changed 5 years ago by
 Description modified (diff)
comment:3 Changed 5 years ago by
 Branch set to u/mmezzarobba/21194reverse
 Commit set to 160f6b35e9253d82362c88aaf82b28cc7dfdc539
 Description modified (diff)
 Milestone set to sage7.4
 Status changed from new to needs_review
 Summary changed from Polynomial.reverse() over ℚ inconsistent with the generic implementation to Multiple bugs in Polynomial.reverse(degree)
comment:4 Changed 5 years ago by
 Commit changed from 160f6b35e9253d82362c88aaf82b28cc7dfdc539 to 08cee2465de3ac7c8289742705ad117a41d37430
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
08cee24  #21194 Fix multiple bugs in Polynomial.reverse()

comment:5 Changed 5 years ago by
 Reviewers set to Vincent Delecroix
 Status changed from needs_review to needs_work
sage: x = polygen(QQ) sage: (x+1).reverse(2**641) 0
Your casting should be <unsigned long> (degree + 1)
.
comment:6 Changed 5 years ago by
 Commit changed from 08cee2465de3ac7c8289742705ad117a41d37430 to 554500b7cecafee1c8596bc4a5c0300a5e82a89b
Branch pushed to git repo; I updated commit sha1. New commits:
554500b  #21194 Fix handling of incorrect input

comment:7 Changed 5 years ago by
 Status changed from needs_work to needs_review
comment:8 Changed 5 years ago by
 Status changed from needs_review to positive_review
comment:9 Changed 5 years ago by
Thanks!
comment:10 Changed 5 years ago by
 Branch changed from u/mmezzarobba/21194reverse to 554500b7cecafee1c8596bc4a5c0300a5e82a89b
 Resolution set to fixed
 Status changed from positive_review to closed
New commits:
Allow ringspecific polynomial rootfinders to pass
#21194 Fix multiple bugs in Polynomial.reverse()