Opened 12 years ago

Closed 12 years ago

#10475 closed defect (fixed)

Calling a polynomial over finite non-prime field with named arguments

Reported by: SimonKing Owned by: AlexGhitza
Priority: major Milestone: sage-4.6.2
Component: basic arithmetic Keywords: Conversion, polynomial
Cc: Merged in: sage-4.6.2.alpha1
Authors: Simon King Reviewers: David Roe, Aly Deines
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description

At sage-support, Thomas pointed to a bug that boils down as follows:

sage: F.<x> = GF(4)
sage: P.<y> = F[]
sage: p = P.random_element(5,5)
sage: SR(p)
Traceback (most recent call last):
...
TypeError: __call__() takes exactly 1 positional argument (0 given)

Reading the _symbolic_ method, one is pointed to the subs method, which is

    def subs(self, *x, **kwds):
        if len(x) == 1 and isinstance(x[0], dict):
            g = self.parent().gen()
            if x[0].has_key(g):
                return self(x[0][g])
            elif len(x[0]) > 0:
                raise TypeError, "keys do not match self's parent"
            return self
        return self.__call__(*x, **kwds)

and there is the error again:

sage: p(y=y)
Traceback (most recent call last):
...
TypeError: __call__() takes exactly 1 positional argument (0 given)

Indeed, the call method of p is

    def __call__(self, a):
        cdef ntl_ZZ_pE _a
        cdef ZZ_pE_c c_b

        K = self._parent.base_ring()

        try:
            if a.parent() is not K:
                a = K.coerce(a)
        except (TypeError, AttributeError, NotImplementedError):
            return Polynomial.__call__(self, a)

        _a = self._parent._modulus.ZZ_pE(list(a.polynomial()))
        ZZ_pEX_eval(c_b, self.x, _a.x)

        R = K.polynomial_ring()
        return K(str(R(ZZ_pE_c_to_list(c_b))))

So, my patch will change the call method so that the arguments can either a single positional argument or one named argument (using the variable name).

Attachments (1)

10475univariate_polynomial_call.patch (2.5 KB) - added by SimonKing 12 years ago.

Download all attachments as: .zip

Change History (9)

comment:1 Changed 12 years ago by SimonKing

  • Authors set to Simon King
  • Milestone set to sage-4.6.1
  • Status changed from new to needs_review

With the patch, one obtains

sage: F.<x> = GF(4)
sage: P.<y> = F[]
sage: p = y^4 + x*y^3 + y^2 + (x + 1)*y + x + 1
sage: SR(p)      #indirect doctest
(((y + x)*y + 1)*y + x + 1)*y + x + 1

That example is added as a test.

comment:2 Changed 12 years ago by roed

  • Status changed from needs_review to needs_work

There's one typo:

raise TypeError, "%s__call__() accepts no named argument except 'x'"%(type(self),self.variable_name())

needs to have a second formatting character.

Other than that, looks good.

Changed 12 years ago by SimonKing

comment:3 Changed 12 years ago by SimonKing

  • Status changed from needs_work to needs_review

Hi,

Thanks for finding it!

Actually there was another bug in it: When no positional argument together with a named argument different from the variable name was provided, then the "wrong" error was raised.

This is fixed in the updated patch, and I also added tests that demonstrate both errors:

            sage: p(2)
            x + 1
            sage: p(y=2)
            x + 1
            sage: p(3,y=2)
            Traceback (most recent call last):
            ...
            TypeError: <type 'sage.rings.polynomial.polynomial_zz_pex.Polynomial_ZZ_pEX'>__call__() takes exactly 1 argument
            sage: p(x=2)
            Traceback (most recent call last):
            ...
            TypeError: <type 'sage.rings.polynomial.polynomial_zz_pex.Polynomial_ZZ_pEX'>__call__() accepts no named argument except 'y'

comment:4 follow-up: Changed 12 years ago by roed

  • Reviewers set to David Roe

Looks fine to me. Once the doctests finish, it has a positive review from me.

I'm going to be traveling and not checking in on this for a while; if someone else wants to finalize it once the doctests pass, go for it.

comment:5 in reply to: ↑ 4 Changed 12 years ago by aly.deines

  • Status changed from needs_review to positive_review

Replying to roed:

Looks fine to me. Once the doctests finish, it has a positive review from me.

I'm going to be traveling and not checking in on this for a while; if someone else wants to finalize it once the doctests pass, go for it.

The doctests pass for me.

comment:6 Changed 12 years ago by jdemeyer

  • Milestone changed from sage-4.6.1 to sage-4.6.2

comment:7 Changed 12 years ago by aly.deines

  • Reviewers changed from David Roe to David Roe, Aly Deines

comment:8 Changed 12 years ago by jdemeyer

  • Merged in set to sage-4.6.2.alpha1
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.