Opened 6 years ago
Last modified 2 months ago
#15077 needs_review defect
Inconsistency in polynomial .reverse(n)
Reported by: | defeo | Owned by: | |
---|---|---|---|
Priority: | minor | Milestone: | sage-duplicate/invalid/wontfix |
Component: | commutative algebra | Keywords: | polynomial univariate reverse |
Cc: | mmezzarobba, jhpalmieri | Merged in: | |
Authors: | Reviewers: | ||
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description
The optional argument of the .reverse()
method of univariate polynomials is interpreted inconsistently through different classes.
Rationals interpret is as "length":
sage: _.<x> = QQ[] sage: (x+1).reverse(1) 1 sage: (x).reverse(1) 0
The docstring for generic polynomials (inherited by CC
, number fields, Polynomial_GF2X
, Polynomial_ZZ_pEX
, ...) says:
If an optional degree argument is given the coefficient list will be truncated or zero padded as necessary and the reverse polynomial will have the specified degree.
but the behaviour is inconsistent with it
sage: _.<x> = GF(2)[] sage: (x+1).reverse(1) x + 1 sage: (x).reverse(1) 1 sage: ['reverse' in cl.__dict__ for cl in inspect.getmro(x.__class__)] [False, False, True, False, False, False, False, False, False, False] sage: inspect.getmro(x.__class__)[2] <type 'sage.rings.polynomial.polynomial_element.Polynomial'>
Polynomial_zmod_flint
and Polynomial_integer_dense_flint
have the exact same docstring and behaviour, though they do not inherit .reverse()
from the generic class:
sage: _.<x> = ZZ[] sage: (x+1).reverse(1) x + 1 sage: (x).reverse(1) 1 sage: ['reverse' in cl.__dict__ for cl in inspect.getmro(x.__class__)] [True, True, False, False, False, False, False, False, False] <type 'sage.rings.polynomial.polynomial_integer_dense_flint.Polynomial_integer_dense_flint'>
Reals take no optional argument. The docstring says
Returns x^d f(1/x) where d is the degree of f.
and the behaviour is consistent with it
sage: (x+1).reverse() x + 1.00000000000000 sage: x.reverse() 1.00000000000000
In my opinion the best behaviour is the one of the generic class, but the docstring should be amended to something similar to the last one, which is the proper mathematical definition. The behaviour of rationals should be corrected to conform to the other classes.
Change History (7)
comment:1 Changed 5 years ago by
- Milestone changed from sage-6.1 to sage-6.2
comment:2 Changed 5 years ago by
- Cc mmezzarobba added
comment:3 Changed 5 years ago by
- Milestone changed from sage-6.2 to sage-6.3
comment:4 Changed 5 years ago by
- Milestone changed from sage-6.3 to sage-6.4
comment:5 Changed 3 years ago by
- Stopgaps set to inconsistentAnswer
comment:6 Changed 2 months ago by
- Cc jhpalmieri added
- Milestone changed from sage-6.4 to sage-duplicate/invalid/wontfix
- Status changed from new to needs_review
comment:7 Changed 2 months ago by
- Stopgaps inconsistentAnswer deleted
There remain two issues: unifying the documentation, and also the version for RR
does not allow for an optional argument. Low priority, but I say we keep this open.
This is obsolete.
QQ['x']
now works as the other cases.