Opened 6 years ago
Closed 6 years ago
#21194 closed defect (fixed)
Multiple bugs in Polynomial.reverse(degree)
Reported by: | mmezzarobba | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-7.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 6 years ago by
- Description modified (diff)
comment:2 Changed 6 years ago by
- Description modified (diff)
comment:3 Changed 6 years ago by
- Branch set to u/mmezzarobba/21194-reverse
- Commit set to 160f6b35e9253d82362c88aaf82b28cc7dfdc539
- Description modified (diff)
- Milestone set to sage-7.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 6 years ago by
- Commit changed from 160f6b35e9253d82362c88aaf82b28cc7dfdc539 to 08cee2465de3ac7c8289742705ad117a41d37430
comment:5 Changed 6 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**64-1) 0
Your casting should be <unsigned long> (degree + 1)
.
comment:6 Changed 6 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 6 years ago by
- Status changed from needs_work to needs_review
comment:8 Changed 6 years ago by
- Status changed from needs_review to positive_review
comment:9 Changed 6 years ago by
Thanks!
comment:10 Changed 6 years ago by
- Branch changed from u/mmezzarobba/21194-reverse to 554500b7cecafee1c8596bc4a5c0300a5e82a89b
- Resolution set to fixed
- Status changed from positive_review to closed
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
#21194 Fix multiple bugs in Polynomial.reverse()