#10304: PolynomialRing_field.lagrange_polynomial doesn't always return a
polynomial
-----------------------------------+----------------------------------------
Reporter: mguaypaq | Owner: mguaypaq
Type: defect | Status: needs_review
Priority: major | Milestone: sage-4.6.1
Component: algebra | Keywords: polynomial ring, lagrange
polynomial
Author: Mathieu Guay-Paquet | Upstream: N/A
Reviewer: Minh Van Nguyen | Merged:
Work_issues: |
-----------------------------------+----------------------------------------
Changes (by mvngu):
* reviewer: => Minh Van Nguyen
Old description:
> The function {{{PolynomialRing_field.lagrange_polynomial}}} sometimes
> returns an element of the coefficient ring or even a python int instead
> of an element of the polynomial ring. This can lead to problems if the
> caller was expecting a polynomial.
>
> {{{
> sage: R, t = FractionField(QQ['t']).objgen(); R.rename('R')
> sage: S, x = R['x'].objgen(); S.rename('S')
> sage: S.lagrange_polynomial([(2, 3 * t), (4, t + 1)]).parent()
> S
> sage: S.lagrange_polynomial([(2, 3 * t)]).parent()
> R
> sage: type(S.lagrange_polynomial([]))
> <type 'int'>
> }}}
>
> The return value is a python int if the list of points was empty, an
> element of the base ring if the list of points had one point, and an
> element of the polynomial ring if the list of points had more than one
> point.
>
> There are similar problems with the base case for the
> {{{algorithm='neville'}}} version:
>
> {{{
> sage: S.lagrange_polynomial([(2, 3 * t), (4, t + 1)],
> algorithm='neville')
> [t + 1, (-t + 1/2)*x + 5*t - 1]
> sage: [poly.parent() for poly in _]
> [R, S]
> sage: S.lagrange_polynomial([(2, 3 * t)], algorithm='neville')
> [3*t]
> sage: [poly.parent() for poly in _]
> [R]
> sage: S.lagrange_polynomial([], algorithm='neville')
> 0
> sage: type(_)
> <type 'int'>
> }}}
>
> Note that in this case, the return value is not even a list when the list
> of points was empty.
New description:
The function {{{PolynomialRing_field.lagrange_polynomial}}} sometimes
returns an element of the coefficient ring or even a python int instead of
an element of the polynomial ring. This can lead to problems if the caller
was expecting a polynomial.
{{{
sage: R, t = FractionField(QQ['t']).objgen(); R.rename('R')
sage: S, x = R['x'].objgen(); S.rename('S')
sage: S.lagrange_polynomial([(2, 3 * t), (4, t + 1)]).parent()
S
sage: S.lagrange_polynomial([(2, 3 * t)]).parent()
R
sage: type(S.lagrange_polynomial([]))
<type 'int'>
}}}
The return value is a python int if the list of points was empty, an
element of the base ring if the list of points had one point, and an
element of the polynomial ring if the list of points had more than one
point.
There are similar problems with the base case for the
{{{algorithm='neville'}}} version:
{{{
sage: S.lagrange_polynomial([(2, 3 * t), (4, t + 1)], algorithm='neville')
[t + 1, (-t + 1/2)*x + 5*t - 1]
sage: [poly.parent() for poly in _]
[R, S]
sage: S.lagrange_polynomial([(2, 3 * t)], algorithm='neville')
[3*t]
sage: [poly.parent() for poly in _]
[R]
sage: S.lagrange_polynomial([], algorithm='neville')
0
sage: type(_)
<type 'int'>
}}}
Note that in this case, the return value is not even a list when the list
of points was empty.
'''Apply:'''
1. [attachment:trac_10304_lagrange_poly_in_self.patch]
1. [attachment:trac-10304_reviewer.patch]
--
Comment:
I'm happy with most of
[attachment:trac_10304_lagrange_poly_in_self.patch], but here are some
problems with that patch:
* A standard idiom in Python is to avoid using a mutable object for a
default value. Using a mutable object (such as a list) as a default value
can introduce subtle and difficult to track bugs. See
[http://effbot.org/zone/default-values.htm this blog post] for an
explanation of why one should avoid using mutable values for default
arguments. See this section in the Python Language Reference for an
official explanation about avoiding the use of mutable objects for default
values.
* When you provide doctests that purports to illustrate that a bug has
been fixed, you need to provide the corresponding ticket number in the
documentation of the doctests.
* The docstring you introduce doesn't use proper ReST syntax.
The reviewer patch [attachment:trac-10304_reviewer.patch] fixes all of the
above issues. If you're OK with my patch, then set the ticket to positive
review. See the ticket description for instructions on which patches to
apply and in which order.
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/10304#comment:2>
Sage <http://www.sagemath.org>
Sage: Creating a Viable Open Source Alternative to Magma, Maple, Mathematica,
and MATLAB
--
You received this message because you are subscribed to the Google Groups
"sage-trac" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to
[email protected].
For more options, visit this group at
http://groups.google.com/group/sage-trac?hl=en.
