fixed polynomial parsing, added doctest

src/sage/modular/modform/ring.py

@@ -29,11 +29,9 @@
 from .constructor                 import ModularForms
 from .element import is_ModularFormElement, GradedModularFormElement
 from .space import is_ModularFormsSpace
-from .defaults import DEFAULT_VARIABLE
 from random import shuffle
 
 from sage.rings.polynomial.multi_polynomial import MPolynomial
-from sage.rings.polynomial.polynomial_element import Polynomial
 from sage.rings.polynomial.polynomial_ring_constructor import PolynomialRing
 from sage.rings.polynomial.term_order import TermOrder
 from sage.rings.power_series_poly import PowerSeries_poly
@@ -176,6 +174,20 @@ class ModularFormsRing(Parent):
         q^3 + 66*q^7 + 832*q^9 + O(q^10),
         q^4 + 40*q^6 + 528*q^8 + O(q^10),
         q^5 + 20*q^7 + 190*q^9 + O(q^10)]
+
+    Elements of modular forms ring can be initiated via multivariate polynomials (see :meth:`from_polynomial`)::
+
+        sage: M = ModularFormsRing(1)
+        sage: M.ngens()
+        2
+        sage: E4, E6 = polygens(QQ, 'E4, E6')
+        sage: M(E4)
+        1 + 240*q + 2160*q^2 + 6720*q^3 + 17520*q^4 + 30240*q^5 + O(q^6)
+        sage: M(E6)
+        1 - 504*q - 16632*q^2 - 122976*q^3 - 532728*q^4 - 1575504*q^5 + O(q^6)
+        sage: M((E4^3 - E6^2)/1728)
+        q - 24*q^2 + 252*q^3 - 1472*q^4 + 4830*q^5 + O(q^6)
+
     """
 
     Element = GradedModularFormElement
@@ -374,24 +386,28 @@ def _generators_variables_dictionnary(self, poly_parent, gens):
         if poly_parent.base_ring() != self.base_ring():
             raise ValueError('the base ring of `poly_parent` must be the same as the base ring of the modular forms ring')
         nb_var = poly_parent.ngens()
-        if nb_var > self.ngens():
-            raise ValueError('the number of variables of the given polynomial ring (%s) cannot exceed the number of generators of the modular forms ring (%s)'%(nb_var, self.ngens()))
+        nb_gens = self.ngens()
+        if nb_var != nb_gens:
+            raise ValueError('the number of variables (%s) must be equal to the number of generators of the modular forms ring (%s)'%(nb_var, self.ngens()))
         return {poly_parent.gen(i) : self(gens[i]) for i in range(0, nb_var)}
 
-    def from_polynomial(self, pol, gens=None):
+    def from_polynomial(self, polynomial, gens=None):
         r"""
-        Convert the given polynomial ``pol`` to a graded form living in ``self``. If
-        ``gens`` is ``None`` then the list of generators given by the method :meth:`gen_forms`
-        will be used. Otherwise, ``gens`` should be a list of generators.
+        Convert the given polynomial to a graded form living in ``self``. If
+        ``gens`` is ``None`` then the list of generators given by the method
+        :meth:`gen_forms` will be used. Otherwise, ``gens`` should be a list of
+        generators.
 
         INPUT:
 
-        - ``pol`` -- A multivariate polynomial. The variables names of the
-          polynomial should be different from ``'q'``.
+        - ``polynomial`` -- A multivariate polynomial. The variables names of
+          the polynomial should be different from ``'q'``. The number of
+          variable of this polynomial should equal the number of generators
         - ``gens`` -- list (default: ``None``) of generators of the modular
           forms ring
 
-        OUTPUT: A GradedModularFormElement given by the polynomial relation ``pol``.
+        OUTPUT: A ``GradedModularFormElement`` given by the polynomial
+        relation ``polynomial``.
 
         EXAMPLES::
 
@@ -411,15 +427,13 @@ def from_polynomial(self, pol, gens=None):
             sage: M.from_polynomial(P(1/2))
             1/2
 
-        The variable names should be different from ``'q'``::
+        Note that the number of variables must be equal to the number of generators::
 
-            sage: M = ModularFormsRing(1)
-            sage: q, t = polygens(QQ, 'q, t')
-            sage: M.from_polynomial(q + t)
+            sage: x, y = polygens(QQ, 'x, y')
+            sage: M(x + y)
             Traceback (most recent call last):
             ...
-            ValueError: the variable name `q` is reserved for q-expansion, please use a different variable name
-
+            ValueError: the number of variables (2) must be equal to the number of generators of the modular forms ring (3)
 
         TESTS::
 
@@ -428,29 +442,21 @@ def from_polynomial(self, pol, gens=None):
             Traceback (most recent call last):
             ...
             NotImplementedError: conversion from polynomial is not implemented if the base ring is not Q
-            sage: M = ModularFormsRing(1)
-            sage: q = polygen(QQ, 'q')
-            sage: M(q^2 + q + 1)
-            Traceback (most recent call last):
-            ...
-            ValueError: the variable name `q` is reserved for q-expansion, please use a different variable name
 
         ..TODO::
 
             * add conversion for symbolic expressions?
         """
         if not self.base_ring() == QQ: # this comes from the method gens_form
             raise NotImplementedError("conversion from polynomial is not implemented if the base ring is not Q")
-        if not isinstance(pol, (MPolynomial, Polynomial)):
-            raise TypeError('`pol` must be a polynomial')
-        if DEFAULT_VARIABLE in pol.parent()._names:
-            raise ValueError('the variable name `q` is reserved for q-expansion, please use a different variable name')
-        if pol.is_constant():
-            return self(pol.constant_coefficient())
+        if not isinstance(polynomial, MPolynomial):
+            raise TypeError('`polynomial` must be a multivariate polynomial')
         if gens is None:
             gens = self.gen_forms()
-        dict = self._generators_variables_dictionnary(pol.parent(), gens)
-        return pol.substitute(dict)
+        dict = self._generators_variables_dictionnary(polynomial.parent(), gens)
+        if polynomial.is_constant():
+            return self(polynomial.constant_coefficient())
+        return polynomial.substitute(dict)
 
     def _element_constructor_(self, forms_datum):
         r"""
@@ -489,7 +495,7 @@ def _element_constructor_(self, forms_datum):
             sage: M('x')
             Traceback (most recent call last):
             ...
-            TypeError: the defining data structure should be a single modular form, a ring element, a list of modular forms, a polynomial or a dictionary
+            TypeError: the defining data structure should be a single modular form, a ring element, a list of modular forms, a multivariate polynomial or a dictionary
             sage: P.<t> = PowerSeriesRing(QQ)
             sage: e = 1 + 240*t + 2160*t^2 + 6720*t^3 + 17520*t^4 + 30240*t^5 + O(t^6)
             sage: ModularFormsRing(1)(e)
@@ -508,12 +514,12 @@ def _element_constructor_(self, forms_datum):
                 raise ValueError('the group (%s) and/or the base ring (%s) of the given modular form is not consistant with the base space: %s'%(forms_datum.group(), forms_datum.base_ring(), self))
         elif forms_datum in self.base_ring():
             forms_dictionary = {0:forms_datum}
-        elif isinstance(forms_datum, (Polynomial, MPolynomial)):
+        elif isinstance(forms_datum, MPolynomial):
             return self.from_polynomial(forms_datum)
         elif isinstance(forms_datum, PowerSeries_poly):
             raise NotImplementedError("conversion from q-expansion not yet implemented")
         else:
-            raise TypeError('the defining data structure should be a single modular form, a ring element, a list of modular forms, a polynomial or a dictionary')
+            raise TypeError('the defining data structure should be a single modular form, a ring element, a list of modular forms, a multivariate polynomial or a dictionary')
         return self.element_class(self, forms_dictionary)
 
     def zero(self):