Inconsistency in polynomial .reverse(n)

[defeo]

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.

CC: @mezzarobba @jhpalmieri

Component: commutative algebra

Keywords: polynomial univariate reverse

Author: Bruno Grenet

Branch/Commit: bd32a84

Reviewer: Frédéric Chapoton

Issue created by migration from https://trac.sagemath.org/ticket/15077

[sagetrac-jakobkroeker]

Stopgaps: inconsistentAnswer

[fchapoton]

comment:6

This is obsolete. QQ['x'] now works as the other cases.

[jhpalmieri]

comment:7

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.

[jhpalmieri]

Changed stopgaps from inconsistentAnswer to none

[bgrenet]

comment:8

There remained actually more issues. The issue for QQ['x'] was indeed fixed by ticket #21194 but that ticket only fixed some of the inconsistencies. Those that I have noticed and fix are:

• No degree parameter for polynomials over RR (as already mentioned);
• No degree parameter for polynomials over Zmod(...) using NTL;
• Several inconsistencies in the documentation (polynomials over ZZ and Zmod(...) using Flint, polynomials over p-adics).

I hope I didn't forgot anything!

[bgrenet]

Author: Bruno Grenet

[bgrenet]

Branch: u/bruno/15077_reverse_consistency

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

64c41ef	15077: reverse for real polynomial with optional parameter
1fc2a6e	15077: Update docstring+tests for Flint (integer & zmod)
7fec024	15077: Make reverse consistent for NTL rings
bd32a84	15077: Make reverse for poly over padics consistent with other implementations

[sagetrac-git]

Commit: bd32a84

[fchapoton]

comment:10

ok, let it be

[fchapoton]

Reviewer: Frédéric Chapoton

[vbraun]

Changed branch from u/bruno/15077_reverse_consistency to bd32a84