Trac #23338: Clean up polynomial constructor

This ticket cleans up the `PolynomialRing()` function in many ways.
Mainly:

1. Use `normalize_names()` as much as possible to deal with variable
names.

2. Fix indentation in docstring.

3. Pass arguments like `sparse` and `implementation` as `**kwds` to the
single-variate or multi-variate polynomial constructor.

4. Make the code easier to understand.

5. Allow `PolynomialRing(..., implementation="singular")` which forces
Singular. This allows simplifying some code in
`src/sage/algebras/free_algebra.py` which currently needs to jump
through hoops to get a `MPolynomialRing_libsingular`.

6. Check more error conditions, for example currently we have
{{{
sage: PolynomialRing(QQ, name="x", names="y")
Univariate Polynomial Ring in y over Rational Field
}}}

7. Change some arguments of `PolynomialRing()` to keyword-only
arguments. This does break some existing uses of `PolynomialRing()`,
although much more likely in library code and not user code (some code
in Sage gets this even wrong and was passing `sparse="lex"` instead of
`order="lex"`)

Apart from items 6 and 7, no existing functionality is changed.

URL: https://trac.sagemath.org/23338
Reported by: jdemeyer
Ticket author(s): Jeroen Demeyer
Reviewer(s): Travis Scrimshaw

Author: Release Manager

src/doc/en/constructions/polynomials.rst

@@ -114,8 +114,7 @@ This example illustrates multivariate polynomial GCD's:
 
 ::
 
-    sage: R = PolynomialRing(RationalField(),3, ['x','y','z'], 'lex')
-    sage: x,y,z = PolynomialRing(RationalField(),3, ['x','y','z'], 'lex').gens()
+    sage: R.<x,y,z> = PolynomialRing(RationalField(), order='lex')
     sage: f = 3*x^2*(x+y)
     sage: g = 9*x*(y^2 - x^2)
     sage: f.gcd(g)

src/sage/algebras/free_algebra.py

@@ -116,17 +116,21 @@
     sage: FreeAlgebra(FreeAlgebra(ZZ,2,'ab'), 2, 'x', implementation='letterplace')
     Traceback (most recent call last):
     ...
-    NotImplementedError: The letterplace implementation is not available for the free algebra you requested
-
+    TypeError: The base ring Free Algebra on 2 generators (a, b) over Integer Ring is not a commutative ring
 """
+
 #*****************************************************************************
-#  Copyright (C) 2005 David Kohel <[email protected]>
-#  Copyright (C) 2005,2006 William Stein <[email protected]>
-#  Copyright (C) 2011 Simon King <[email protected]>
+#       Copyright (C) 2005 David Kohel <[email protected]>
+#       Copyright (C) 2005,2006 William Stein <[email protected]>
+#       Copyright (C) 2011 Simon King <[email protected]>
 #
-#  Distributed under the terms of the GNU General Public License (GPL)
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 2 of the License, or
+# (at your option) any later version.
 #                  http://www.gnu.org/licenses/
 #*****************************************************************************
+
 from __future__ import absolute_import
 from six.moves import range
 from six import integer_types
@@ -139,13 +143,10 @@
 
 from sage.algebras.free_algebra_element import FreeAlgebraElement
 
-import sage.structure.parent_gens
-
 from sage.structure.factory import UniqueFactory
 from sage.misc.cachefunc import cached_method
 from sage.all import PolynomialRing
 from sage.rings.ring import Algebra
-from sage.rings.polynomial.multi_polynomial_libsingular import MPolynomialRing_libsingular
 from sage.categories.algebras_with_basis import AlgebrasWithBasis
 from sage.combinat.free_module import CombinatorialFreeModule
 from sage.combinat.words.word import Word
@@ -266,21 +267,14 @@ def create_key(self,base_ring, arg1=None, arg2=None,
             return tuple(degrees),base_ring
         PolRing = None
         # test if we can use libSingular/letterplace
-        if implementation is not None and implementation != 'generic':
-            try:
-                PolRing = PolynomialRing(base_ring, arg1, arg2,
-                                   sparse=sparse, order=order,
-                                   names=names, name=name,
-                                   implementation=implementation if implementation != 'letterplace' else None)
-                if not isinstance(PolRing, MPolynomialRing_libsingular):
-                    if PolRing.ngens() == 1:
-                        PolRing = PolynomialRing(base_ring, 1, PolRing.variable_names())
-                        if not isinstance(PolRing, MPolynomialRing_libsingular):
-                            raise TypeError
-                    else:
-                        raise TypeError
-            except (TypeError, NotImplementedError) as msg:
-                raise NotImplementedError("The letterplace implementation is not available for the free algebra you requested")
+        if implementation == "letterplace":
+            args = [arg for arg in (arg1, arg2) if arg is not None]
+            kwds = dict(sparse=sparse, order=order, implementation="singular")
+            if name is not None:
+                kwds["name"] = name
+            if names is not None:
+                kwds["names"] = names
+            PolRing = PolynomialRing(base_ring, *args, **kwds)
         if PolRing is not None:
             if degrees is None:
                 return (PolRing,)

src/sage/algebras/letterplace/free_algebra_letterplace.pyx

@@ -173,7 +173,9 @@ cdef MPolynomialRing_libsingular make_letterplace_ring(base_ring,blocks):
     for i from 1<=i<blocks:
         T += T0
         names.extend([x+'_'+str(i) for x in names0])
-    return PolynomialRing(base_ring.base_ring(),len(names),names,order=T)
+    return PolynomialRing(base_ring.base_ring(), names, order=T,
+            implementation="singular")
+
 
 #####################
 # The free algebra

src/sage/interfaces/singular.py

@@ -1640,13 +1640,13 @@ def sage_poly(self, R=None, kcache=None):
 
         EXAMPLES::
 
-            sage: R = PolynomialRing(GF(2^8,'a'),2,'xy')
-            sage: f=R('a^20*x^2*y+a^10+x')
-            sage: f._singular_().sage_poly(R)==f
+            sage: R = PolynomialRing(GF(2^8,'a'), 'x,y')
+            sage: f = R('a^20*x^2*y+a^10+x')
+            sage: f._singular_().sage_poly(R) == f
             True
-            sage: R = PolynomialRing(GF(2^8,'a'),1,'x')
-            sage: f=R('a^20*x^3+x^2+a^10')
-            sage: f._singular_().sage_poly(R)==f
+            sage: R = PolynomialRing(GF(2^8,'a'), 'x', implementation="singular")
+            sage: f = R('a^20*x^3+x^2+a^10')
+            sage: f._singular_().sage_poly(R) == f
             True
 
         ::

src/sage/libs/singular/function.pyx

@@ -1317,7 +1317,7 @@ cdef class SingularFunction(SageObject):
             if ring is None:
                 if dummy_ring is None:
                     from sage.all import QQ, PolynomialRing
-                    dummy_ring = PolynomialRing(QQ,"dummy",1) # seems a reasonable default
+                    dummy_ring = PolynomialRing(QQ, "dummy", implementation="singular") # seems a reasonable default
                 ring = dummy_ring
         if not (isinstance(ring, MPolynomialRing_libsingular) or isinstance(ring, NCPolynomialRing_plural)):
             raise TypeError("Cannot call Singular function '%s' with ring parameter of type '%s'"%(self._name,type(ring)))

src/sage/libs/singular/ring.pyx

@@ -137,8 +137,8 @@ cdef ring *singular_ring_new(base_ring, n, names, term_order) except NULL:
     _ring  = NULL
 
     n = int(n)
-    if n<1:
-        raise ArithmeticError("The number of variables must be at least 1.")
+    if n < 1:
+        raise NotImplementedError(f"polynomials in {n} variables are not supported in Singular")
 
     nvars = n
     order = TermOrder(term_order, n)
@@ -226,10 +226,8 @@ cdef ring *singular_ring_new(base_ring, n, names, term_order) except NULL:
 
     elif isinstance(base_ring, NumberField) and base_ring.is_absolute():
         characteristic = 1
-        try:
-            k = PolynomialRing(RationalField(), 1, [base_ring.variable_name()], 'lex')
-        except TypeError:
-            raise TypeError("The multivariate polynomial ring in a single variable %s in lex order over Rational Field is supposed to be of type %s" % (base_ring.variable_name(), MPolynomialRing_libsingular))
+        k = PolynomialRing(RationalField(),
+            name=base_ring.variable_name(), order="lex", implementation="singular")
 
         minpoly = base_ring.polynomial()(k.gen())
 
@@ -257,8 +255,6 @@ cdef ring *singular_ring_new(base_ring, n, names, term_order) except NULL:
         _ring = rDefault (_cf ,nvars, _names, nblcks, _order, _block0, _block1, _wvhdl)
 
     elif (isinstance(base_ring, FiniteField_generic) and base_ring.is_prime_field()):
-        #or (is_IntegerModRing(base_ring) and base_ring.characteristic().is_prime()):
-
         if base_ring.characteristic() <= 2147483647:
             characteristic = base_ring.characteristic()
         else:
@@ -274,11 +270,10 @@ cdef ring *singular_ring_new(base_ring, n, names, term_order) except NULL:
             characteristic = -base_ring.characteristic() # note the negative characteristic
         else:
             raise TypeError("characteristic must be <= 2147483647.")
-        # TODO: This is lazy, it should only call Singular stuff not MPolynomial stuff
-        try:
-            k = PolynomialRing(base_ring.prime_subfield(), 1, [base_ring.variable_name()], 'lex')
-        except TypeError:
-            raise TypeError("The multivariate polynomial ring in a single variable %s in lex order over %s is supposed to be of type %s" % (base_ring.variable_name(), base_ring,MPolynomialRing_libsingular))
+
+        # TODO: This is lazy, it should only call Singular stuff not PolynomialRing()
+        k = PolynomialRing(base_ring.prime_subfield(),
+            name=base_ring.variable_name(), order="lex", implementation="singular")
         minpoly = base_ring.polynomial()(k.gen())
 
         ch = base_ring.characteristic()
@@ -307,6 +302,9 @@ cdef ring *singular_ring_new(base_ring, n, names, term_order) except NULL:
     elif is_IntegerModRing(base_ring):
 
         ch = base_ring.characteristic()
+        if ch < 2:
+            raise NotImplementedError(f"polynomials over {base_ring} are not supported in Singular")
+
         isprime = ch.is_prime()
 
         if not isprime and ch.is_power_of(2):
@@ -356,10 +354,8 @@ cdef ring *singular_ring_new(base_ring, n, names, term_order) except NULL:
             _cf = nInitChar( n_Zn, <void *>&_info )
         _ring = rDefault( _cf ,nvars, _names, nblcks, _order, _block0, _block1, _wvhdl)
 
-
     else:
-        raise NotImplementedError("Base ring is not supported.")
-
+        raise NotImplementedError(f"polynomials over {base_ring} are not supported in Singular")
 
     if (_ring is NULL):
         raise ValueError("Failed to allocate Singular ring.")

src/sage/matrix/matrix_generic_sparse.pyx

@@ -160,7 +160,7 @@ cdef class Matrix_generic_sparse(matrix_sparse.Matrix_sparse):
             sage: loads(dumps(m)) == m
             True
 
-            sage: R2.<a,b> = PolynomialRing(QQ,'a','b')
+            sage: R2.<a,b> = PolynomialRing(QQ)
             sage: M2 = MatrixSpace(R2,2,3,sparse=True)
             sage: M2(m)
             [4 1 0]

src/sage/misc/explain_pickle.py

@@ -18,11 +18,11 @@
     pg_make_integer('c1p')
     sage: explain_pickle(dumps(polygen(QQ)))
     pg_Polynomial_rational_flint = unpickle_global('sage.rings.polynomial.polynomial_rational_flint', 'Polynomial_rational_flint')
-    pg_PolynomialRing = unpickle_global('sage.rings.polynomial.polynomial_ring_constructor', 'PolynomialRing')
+    pg_unpickle_PolynomialRing = unpickle_global('sage.rings.polynomial.polynomial_ring_constructor', 'unpickle_PolynomialRing')
     pg_RationalField = unpickle_global('sage.rings.rational_field', 'RationalField')
     pg = unpickle_instantiate(pg_RationalField, ())
     pg_make_rational = unpickle_global('sage.rings.rational', 'make_rational')
-    pg_Polynomial_rational_flint(pg_PolynomialRing(pg, 'x', None, False), [pg_make_rational('0'), pg_make_rational('1')], False, True)
+    pg_Polynomial_rational_flint(pg_unpickle_PolynomialRing(pg, ('x',), None, False), [pg_make_rational('0'), pg_make_rational('1')], False, True)
     sage: sage_eval(explain_pickle(dumps(polygen(QQ)))) == polygen(QQ)
     True
 
@@ -40,8 +40,9 @@
     make_integer('c1p')
     sage: explain_pickle(dumps(polygen(QQ)), in_current_sage=True)
     from sage.rings.polynomial.polynomial_rational_flint import Polynomial_rational_flint
+    from sage.rings.polynomial.polynomial_ring_constructor import unpickle_PolynomialRing
     from sage.rings.rational import make_rational
-    Polynomial_rational_flint(PolynomialRing(RationalField(), 'x', None, False), [make_rational('0'), make_rational('1')], False, True)
+    Polynomial_rational_flint(unpickle_PolynomialRing(RationalField(), ('x',), None, False), [make_rational('0'), make_rational('1')], False, True)
 
 The explain_pickle function has several use cases.
 

src/sage/rings/ideal.py

@@ -1661,7 +1661,7 @@ def Katsura(R, n=None, homog=False, singular=singular_default):
 
     ::
 
-        sage: Q.<x> = PolynomialRing(QQ,1)
+        sage: Q.<x> = PolynomialRing(QQ, implementation="singular")
         sage: J = sage.rings.ideal.Katsura(Q,1); J
         Ideal (x - 1) of Multivariate Polynomial Ring in x over Rational Field
     """

src/sage/rings/polynomial/laurent_polynomial_ring.py

@@ -383,7 +383,7 @@ def _multi_variate(base_ring, names, n, sparse, order):
                 break
         else:
             break
-    R = _multi_variate_poly(base_ring, names, n, sparse, order, None)
+    R = _multi_variate_poly(base_ring, names, sparse, order)
     P = LaurentPolynomialRing_mpair(R, prepend_string, names)
     _save_in_cache(key, P)
     return P

src/sage/rings/polynomial/multi_polynomial_libsingular.pyx

@@ -137,7 +137,7 @@ TESTS::
     sage: loads(dumps(x)) == x
     True
 
-    sage: Rt.<t> = PolynomialRing(QQ,1)
+    sage: Rt.<t> = PolynomialRing(QQ, implementation="singular")
     sage: p = 1+t
     sage: R.<u,v> = PolynomialRing(QQ, 2)
     sage: p(u/v)
@@ -363,6 +363,26 @@ cdef class MPolynomialRing_libsingular(MPolynomialRing_generic):
             Polynomial base injection morphism:
               From: Integer Ring
               To:   Multivariate Polynomial Ring in x, y over Integer Ring
+
+        Check some invalid input::
+
+            sage: from sage.rings.polynomial.multi_polynomial_libsingular import MPolynomialRing_libsingular
+            sage: MPolynomialRing_libsingular(Zmod(1), 1, ["x"], "lex")
+            Traceback (most recent call last):
+            ...
+            NotImplementedError: polynomials over Ring of integers modulo 1 are not supported in Singular
+            sage: MPolynomialRing_libsingular(SR, 1, ["x"], "lex")
+            Traceback (most recent call last):
+            ...
+            NotImplementedError: polynomials over Symbolic Ring are not supported in Singular
+            sage: MPolynomialRing_libsingular(QQ, 0, [], "lex")
+            Traceback (most recent call last):
+            ...
+            NotImplementedError: polynomials in 0 variables are not supported in Singular
+            sage: MPolynomialRing_libsingular(QQ, -1, [], "lex")
+            Traceback (most recent call last):
+            ...
+            ValueError: Multivariate Polynomial Rings must have more than 0 variables.
         """
         MPolynomialRing_generic.__init__(self, base_ring, n, names, order)
         self._has_singular = True
@@ -1947,7 +1967,7 @@ def unpickle_MPolynomialRing_libsingular(base_ring, names, term_order):
     from sage.rings.polynomial.polynomial_ring_constructor import _multi_variate
     # If libsingular would be replaced by a different implementation in future
     # sage version, the unpickled ring will belong the new implementation.
-    return _multi_variate(base_ring, tuple(names), len(names), False, term_order, None)
+    return _multi_variate(base_ring, tuple(names), False, term_order, None)
 
 cdef class MPolynomial_libsingular(MPolynomial):
     """
@@ -3018,7 +3038,7 @@ cdef class MPolynomial_libsingular(MPolynomial):
             sage: f[0,1]
             0
 
-            sage: R.<x> = PolynomialRing(GF(7),1); R
+            sage: R.<x> = PolynomialRing(GF(7), implementation="singular"); R
             Multivariate Polynomial Ring in x over Finite Field of size 7
             sage: f = 5*x^2 + 3; f
             -2*x^2 + 3

src/sage/rings/polynomial/plural.pyx

@@ -2227,7 +2227,7 @@ cdef class NCPolynomial_plural(RingElement):
             sage: f[0,0,0]
             0
 
-            sage: R.<x> = PolynomialRing(GF(7),1); R
+            sage: R.<x> = PolynomialRing(GF(7), implementation="singular"); R
             Multivariate Polynomial Ring in x over Finite Field of size 7
             sage: f = 5*x^2 + 3; f
             -2*x^2 + 3

src/sage/rings/polynomial/polynomial_ring.py

@@ -120,7 +120,7 @@
 
 Check that :trac:`5562` has been fixed::
 
-    sage: R.<u> = PolynomialRing(RDF, 1, 'u')
+    sage: R.<u> = PolynomialRing(RDF, 1)
     sage: v1 = vector([u])
     sage: v2 = vector([CDF(2)])
     sage: v1 * v2
@@ -210,9 +210,7 @@ def is_PolynomialRing(x):
 
     ::
 
-        sage: is_PolynomialRing(PolynomialRing(ZZ,1,'w'))
-        False
-        sage: R = PolynomialRing(ZZ,1,'w'); R
+        sage: R.<w> = PolynomialRing(ZZ, implementation="singular"); R
         Multivariate Polynomial Ring in w over Integer Ring
         sage: is_PolynomialRing(R)
         False
@@ -294,10 +292,9 @@ def __init__(self, base_ring, name=None, sparse=False, element_class=None, categ
                 self._Karatsuba_threshold = 8
 
     def __reduce__(self):
-        import sage.rings.polynomial.polynomial_ring_constructor
-        return (sage.rings.polynomial.polynomial_ring_constructor.PolynomialRing,
-                (self.base_ring(), self.variable_name(), None, self.is_sparse()))
-
+        from sage.rings.polynomial.polynomial_ring_constructor import unpickle_PolynomialRing
+        args = (self.base_ring(), self.variable_names(), None, self.is_sparse())
+        return unpickle_PolynomialRing, args
 
     def _element_constructor_(self, x=None, check=True, is_gen=False,
                               construct=False, **kwds):
@@ -1598,7 +1595,7 @@ def __init__(self, base_ring, name="x", sparse=False, implementation=None,
                     element_class = Polynomial_integer_dense_flint
                     self._implementation_names = (None, 'FLINT')
                 else:
-                    raise ValueError("Unknown implementation %s for ZZ[x]"%implementation)
+                    raise ValueError("unknown implementation %r for ZZ[%r]" % (implementation, name[0]))
         PolynomialRing_commutative.__init__(self, base_ring, name=name,
                 sparse=sparse, element_class=element_class, category=category)