Opened 4 years ago

Closed 22 months ago

#17554 closed defect (fixed)

Univariate Laurent polynomial do not properly handle __call__

Reported by: tscrim Owned by: tscrim
Priority: major Milestone: sage-8.0
Component: commutative algebra Keywords: Laurent polynomial, substitution
Cc: Merged in:
Authors: Travis Scrimshaw Reviewers: Frédéric Chapoton
Report Upstream: N/A Work issues:
Branch: 7f41391 (Commits) Commit: 7f413919fac082f54909ed906bd430cc45b7bbd7
Dependencies: Stopgaps:

Description (last modified by tscrim)

Univariate Laurent polynomials behave very differently with __call__ compared to other polynomials. In particular, the following does not (correctly) work:

sage: R.<t> = LaurentPolynomialRing(ZZ)
sage: f = t^(-2) + t^2
sage: f(t=-1)  # Boom
sage: f(x=-1)  # Boom
sage: f()  # Boom
sage: f(1,2)  # Should be an error
2

The original symptom (which has been fixed by other means, see comment:3) came from

sage: R.<q> = QQ[]
sage: p = q^4 + q^2 - 2*q + 3
sage: L.<x,y> = LaurentPolynomialRing(QQ)
sage: p(q=x)
x^4 + x^2 - 2*x + 3

but if we change things to a univariate Laurent polynomial ring, we get:

sage: L.<x> = LaurentPolynomialRing(QQ)
sage: p(q=x)
...
IndexError: tuple index out of range

See comment:2.

Change History (6)

comment:1 Changed 4 years ago by tscrim

  • Summary changed from Unable to substitute univariant Laurent polynomial into usual polynomial to Unable to substitute univariate Laurent polynomial into usual polynomial

comment:2 Changed 4 years ago by nbruin

The following seems to be the issue

sage: p() #evaluating with no arguments should probably be a no-op
q^4 + q^2 - 2*q + 3
sage: x() #but it raises an error for laurent polynomials
IndexError: tuple index out of range

The problem gets triggered by sage.rings.polynomial.polynomial_element.Polynomial.__call__, which for keyword arguments does:

          P = self.parent()
            name = P.variable_name()
            if name in kwds:
                if len(x) > 0:
                    raise ValueError("must not specify both a keyword and positional argument")
                a = self(kwds[name])
                del kwds[name]
                try:
                    return a(**kwds)
                except TypeError:
                    return a

The command return a(**kwds) fails, because it's effectively a(*(),**{}). Furthermore, it fails with an IndexError which doesn't get caught.

The better solution is probably to amend laurentpolynomial's __call__ to do the right thing. Presently, it doesn't support keyword arguments at all and it expects non-empty arguments. Its implementation is

    def __call__(self, *x):
        if isinstance(x[0], tuple):
            x = x[0]
        return self.__u(x) * (x[0]**self.__n)

which expects there is at least one argument and doesn't handle keyword arguments.

comment:3 Changed 22 months ago by chapoton

This seems to work now (8.0.b11)

comment:4 Changed 22 months ago by tscrim

  • Authors set to Travis Scrimshaw
  • Branch set to public/rings/laurent_polynomial_call-17554
  • Commit set to 7f413919fac082f54909ed906bd430cc45b7bbd7
  • Description modified (diff)
  • Milestone changed from sage-6.5 to sage-8.0
  • Status changed from new to needs_review
  • Summary changed from Unable to substitute univariate Laurent polynomial into usual polynomial to Univariate Laurent polynomial do not properly handle __call__

However, there are still serious issues with __call__ that I am recycling this ticket for (sorry it completely fell off my radar). The attached branch makes the behavior standard with the rest of polynomials Sage (with some mild cleanup of the multivariate __call__).


New commits:

7f41391Make univariate Laurent polynomials behave like other polynomials.

comment:5 Changed 22 months ago by chapoton

  • Reviewers set to Frédéric Chapoton
  • Status changed from needs_review to positive_review

ok, let it be. Thanks

comment:6 Changed 22 months ago by vbraun

  • Branch changed from public/rings/laurent_polynomial_call-17554 to 7f413919fac082f54909ed906bd430cc45b7bbd7
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.