Trac #33757: commutativity test

In several places the commutativity of a ring is tested via
`isinstance(base_ring, sage.rings.ring.CommutativeRing)` rather than
`base_ring in sage.categories.commutative_rings.CommutativeRings()`.
This makes it impossible to have a nicely interacting ring that would
just inherit from `Parent`.

Note that the answers from `ring.is_commutative()` and `ring in
CommutativeRings()` could be different : the former tests the category
initialization whether the second could involve some (possibly costly)
checks.

The same remark applies to
- integral domains
- fields
- ...

See also #32810

URL: https://trac.sagemath.org/33757
Reported by: vdelecroix
Ticket author(s): Frédéric Chapoton
Reviewer(s): Travis Scrimshaw

Author: Release Manager

src/sage/categories/schemes.py

@@ -137,11 +137,11 @@ def _call_(self, x):
         from sage.schemes.generic.morphism import is_SchemeMorphism
         if is_SchemeMorphism(x):
             return x
-        from sage.rings.ring import CommutativeRing
+        from sage.categories.commutative_rings import CommutativeRings
         from sage.schemes.generic.spec import Spec
         from sage.categories.map import Map
         from sage.categories.all import Rings
-        if isinstance(x, CommutativeRing):
+        if x in CommutativeRings():
             return Spec(x)
         elif isinstance(x, Map) and x.category_for().is_subcategory(Rings()):
             # x is a morphism of Rings

src/sage/dynamics/arithmetic_dynamics/wehlerK3.py

@@ -27,13 +27,12 @@
 
 from sage.calculus.functions import jacobian
 from sage.categories.fields import Fields
+from sage.categories.commutative_rings import CommutativeRings
 from sage.categories.number_fields import NumberFields
 from sage.misc.functional import sqrt
 from sage.misc.cachefunc import cached_method
 from sage.misc.mrange import xmrange
-from sage.rings.all import CommutativeRing
 from sage.rings.finite_rings.finite_field_constructor import GF
-from sage.rings.finite_rings.finite_field_base import is_FiniteField
 from sage.rings.fraction_field import FractionField
 from sage.rings.integer_ring import ZZ
 from sage.rings.number_field.order import is_NumberFieldOrder
@@ -48,6 +47,7 @@
 
 _NumberFields = NumberFields()
 _Fields = Fields()
+_CommutativeRings = CommutativeRings()
 
 
 def WehlerK3Surface(polys):
@@ -75,14 +75,12 @@ def WehlerK3Surface(polys):
 
     R = polys[0].parent().base_ring()
     if R in _Fields:
-        if is_FiniteField(R):
+        if R in _Fields.Finite():
             return WehlerK3Surface_finite_field(polys)
-        else:
-            return WehlerK3Surface_field(polys)
-    elif isinstance(R, CommutativeRing):
+        return WehlerK3Surface_field(polys)
+    if R in _CommutativeRings:
         return WehlerK3Surface_ring(polys)
-    else:
-        raise TypeError("R (= %s) must be a commutative ring" % R)
+    raise TypeError("R (= %s) must be a commutative ring" % R)
 
 
 def random_WehlerK3Surface(PP):
@@ -131,7 +129,7 @@ class WehlerK3Surface_ring(AlgebraicScheme_subscheme_product_projective):
           x*y*v^2 + z^2*u*w
     """
     def __init__(self, polys):
-        if not isinstance(polys, (list,tuple)):
+        if not isinstance(polys, (list, tuple)):
             raise TypeError("polys must be a list or tuple of polynomials")
         R = polys[0].parent()
         vars = R.variable_names()
@@ -2295,7 +2293,6 @@ def orbit_phi(self, P, N, **kwds):
             : 3992260691327218828582255586014718568398539828275296031491644987908/18550615454277582153932951051931712107449915856862264913424670784695 :
             1 , -117756062505511/54767410965117 : -23134047983794359/37466994368025041 : 1)]
         """
-
         if not isinstance(N, (list, tuple)):
             N = [0, N]
         try:

src/sage/geometry/polyhedron/parent.py

@@ -19,7 +19,6 @@
 from sage.rings.integer_ring import ZZ
 from sage.rings.rational_field import QQ
 from sage.rings.real_double import RDF
-from sage.rings.ring import CommutativeRing
 from sage.categories.fields import Fields
 from sage.categories.rings import Rings
 from sage.categories.modules import Modules
@@ -1012,7 +1011,7 @@ def _get_action_(self, other, op, self_is_left):
                                            extended_self._internal_coerce_map_from(self).__copy__())
             return action
 
-        if op is operator.mul and isinstance(other, CommutativeRing):
+        if op is operator.mul and other in Rings().Commutative():
             ring = self._coerce_base_ring(other)
             if ring is self.base_ring():
                 return ActedUponAction(other, self, not self_is_left)

src/sage/rings/complex_double.pyx

@@ -174,7 +174,7 @@ cdef class ComplexDoubleField_class(sage.rings.abc.ComplexDoubleField):
             (-1.0, -1.0 + 1.2246...e-16*I, False)
         """
         from sage.categories.fields import Fields
-        ParentWithGens.__init__(self, self, ('I',), normalize=False, category=Fields().Metric().Complete())
+        ParentWithGens.__init__(self, self, ('I',), normalize=False, category=Fields().Infinite().Metric().Complete())
         self._populate_coercion_lists_()
 
     def __reduce__(self):

src/sage/rings/ideal.py

@@ -11,7 +11,7 @@
 If `R` is non-commutative, the former creates a left and the latter
 a right ideal, and ``R*[a,b,...]*R`` creates a two-sided ideal.
 """
-#*****************************************************************************
+# ****************************************************************************
 #       Copyright (C) 2005 William Stein <[email protected]>
 #
 #  Distributed under the terms of the GNU General Public License (GPL)
@@ -23,16 +23,23 @@
 #
 #  The full text of the GPL is available at:
 #
-#                  http://www.gnu.org/licenses/
-#*****************************************************************************
+#                  https://www.gnu.org/licenses/
+# ****************************************************************************
 
 from types import GeneratorType
 
-import sage.rings.ring
+from sage.categories.rings import Rings
+from sage.categories.fields import Fields
 from sage.structure.element import MonoidElement
 from sage.structure.richcmp import rich_to_bool, richcmp
 from sage.structure.sequence import Sequence
 
+
+# for efficiency
+_Rings = Rings()
+_Fields = Fields()
+
+
 def Ideal(*args, **kwds):
     r"""
     Create the ideal in ring with given generators.
@@ -171,7 +178,7 @@ def Ideal(*args, **kwds):
     first = args[0]
 
     inferred_field = False
-    if not isinstance(first, sage.rings.ring.Ring):
+    if first not in _Rings:
         if isinstance(first, Ideal_generic) and len(args) == 1:
             R = first.ring()
             gens = first.gens()
@@ -182,12 +189,12 @@ def Ideal(*args, **kwds):
                 gens = args
             gens = Sequence(gens)
             R = gens.universe()
-            inferred_field = isinstance(R, sage.rings.ring.Field)
+            inferred_field = R in _Fields
     else:
         R = first
         gens = args[1:]
 
-    if not isinstance(R, sage.rings.ring.CommutativeRing):
+    if R not in _Rings.Commutative():
         raise TypeError("R must be a commutative ring")
 
     I = R.ideal(*gens, **kwds)
@@ -200,6 +207,7 @@ def Ideal(*args, **kwds):
 
     return I
 
+
 def is_Ideal(x):
     r"""
     Return ``True`` if object is an ideal of a ring.

src/sage/rings/polynomial/polynomial_ring_constructor.py

@@ -11,18 +11,15 @@
 rings but rather quotients of them (see module
 :mod:`sage.rings.polynomial.pbori` for more details).
 """
-
-#*****************************************************************************
+# ****************************************************************************
 #       Copyright (C) 2006 William Stein <[email protected]>
 #
 # 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/
-#*****************************************************************************
-
-
+#                  https://www.gnu.org/licenses/
+# ****************************************************************************
 from sage.structure.category_object import normalize_names
 import sage.rings.ring as ring
 import sage.rings.padics.padic_base_leaves as padic_base_leaves
@@ -32,16 +29,19 @@
 from sage.rings.finite_rings.finite_field_base import is_FiniteField
 
 from sage.misc.cachefunc import weak_cached_function
+import sage.misc.weak_dict
 
 from sage.categories.fields import Fields
-_Fields = Fields()
 from sage.categories.commutative_rings import CommutativeRings
-_CommutativeRings = CommutativeRings()
+from sage.categories.domains import Domains
 from sage.categories.complete_discrete_valuation import CompleteDiscreteValuationRings, CompleteDiscreteValuationFields
+
+_CommutativeRings = CommutativeRings()
+_Fields = Fields()
+_Domains = Domains()
 _CompleteDiscreteValuationRings = CompleteDiscreteValuationRings()
 _CompleteDiscreteValuationFields = CompleteDiscreteValuationFields()
 
-import sage.misc.weak_dict
 _cache = sage.misc.weak_dict.WeakValueDictionary()
 
 
@@ -659,6 +659,7 @@ def unpickle_PolynomialRing(base_ring, arg1=None, arg2=None, sparse=False):
     args = [arg for arg in (arg1, arg2) if arg is not None]
     return PolynomialRing(base_ring, *args, sparse=sparse)
 
+
 from sage.misc.persist import register_unpickle_override
 register_unpickle_override('sage.rings.polynomial.polynomial_ring_constructor', 'PolynomialRing', unpickle_PolynomialRing)
 
@@ -720,15 +721,15 @@ def _single_variate(base_ring, name, sparse=None, implementation=None, order=Non
 
     # Generic implementations
     if constructor is None:
-        if not isinstance(base_ring, ring.CommutativeRing):
+        if base_ring not in _CommutativeRings:
             constructor = polynomial_ring.PolynomialRing_general
         elif base_ring in _CompleteDiscreteValuationRings:
             constructor = polynomial_ring.PolynomialRing_cdvr
         elif base_ring in _CompleteDiscreteValuationFields:
             constructor = polynomial_ring.PolynomialRing_cdvf
-        elif base_ring.is_field(proof=False):
+        elif base_ring in _Fields:
             constructor = polynomial_ring.PolynomialRing_field
-        elif base_ring.is_integral_domain(proof=False):
+        elif base_ring in _Domains:
             constructor = polynomial_ring.PolynomialRing_integral_domain
         else:
             constructor = polynomial_ring.PolynomialRing_commutative
@@ -737,8 +738,8 @@ def _single_variate(base_ring, name, sparse=None, implementation=None, order=Non
 
         # Only use names which are not supported by the specialized class.
         if specialized is not None:
-            implementation_names = [n for n in implementation_names if
-                    specialized._implementation_names_impl(n, base_ring, sparse) is NotImplemented]
+            implementation_names = [n for n in implementation_names
+                                    if specialized._implementation_names_impl(n, base_ring, sparse) is NotImplemented]
 
     if implementation is not None:
         kwds["implementation"] = implementation
@@ -928,7 +929,6 @@ def BooleanPolynomialRing_constructor(n=None, names=None, order="lex"):
         sage: x2 > x3
         True
     """
-
     if isinstance(n, str):
         names = n
         n = -1

src/sage/rings/quotient_ring.py

@@ -97,17 +97,15 @@
     True
 
 """
-#*****************************************************************************
+# ****************************************************************************
 #       Copyright (C) 2005 William Stein <[email protected]>
 #
 # 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/
-#*****************************************************************************
-
-
+# ****************************************************************************
 import sage.misc.latex as latex
 from . import ring, ideal, quotient_ring_element
 from sage.structure.category_object import normalize_names
@@ -118,6 +116,10 @@
 from sage.categories.commutative_rings import CommutativeRings
 
 
+_Rings = Rings()
+_CommRings = CommutativeRings()
+
+
 MPolynomialIdeal_quotient = None
 try:
     from sage.interfaces.singular import singular as singular_default, is_SingularElement
@@ -276,20 +278,15 @@ def QuotientRing(R, I, names=None, **kwds):
     """
     # 1. Not all rings inherit from the base class of rings.
     # 2. We want to support quotients of free algebras by homogeneous two-sided ideals.
-    #if not isinstance(R, commutative_ring.CommutativeRing):
-    #    raise TypeError, "R must be a commutative ring."
     from sage.rings.finite_rings.integer_mod_ring import Integers
     from sage.rings.integer_ring import ZZ
-    if R not in Rings():
-        raise TypeError("R must be a ring.")
-    try:
-        is_commutative = R.is_commutative()
-    except (AttributeError, NotImplementedError):
-        is_commutative = False
+    if R not in _Rings:
+        raise TypeError("R must be a ring")
+    is_commutative = R in _CommRings
     if names is None:
         try:
             names = tuple([x + 'bar' for x in R.variable_names()])
-        except ValueError: # no names are assigned
+        except ValueError:  # no names are assigned
             pass
     else:
         names = normalize_names(R.ngens(), names)
@@ -321,10 +318,11 @@ def QuotientRing(R, I, names=None, **kwds):
         if S == ZZ:
             return Integers((I_lift+J).gen(), **kwds)
         return R.__class__(S, I_lift + J, names=names)
-    if isinstance(R, ring.CommutativeRing):
+    if R in _CommRings:
         return QuotientRing_generic(R, I, names, **kwds)
     return QuotientRing_nc(R, I, names, **kwds)
 
+
 def is_QuotientRing(x):
     """
     Tests whether or not ``x`` inherits from :class:`QuotientRing_nc`.
@@ -349,14 +347,12 @@ def is_QuotientRing(x):
         True
         sage: is_QuotientRing(F)
         False
-
     """
     return isinstance(x, QuotientRing_nc)
 
 
-_Rings = Rings()
-_RingsQuotients = Rings().Quotients()
-_CommutativeRingsQuotients = CommutativeRings().Quotients()
+_RingsQuotients = _Rings.Quotients()
+_CommutativeRingsQuotients = _CommRings.Quotients()
 from sage.structure.category_object import check_default_category
 
 
@@ -512,10 +508,13 @@ def construction(self):
                 names = self.cover_ring().variable_names()
             except ValueError:
                 names = None
-        if self in CommutativeRings():
-            return QuotientFunctor(self.__I, names=names, domain=CommutativeRings(), codomain=CommutativeRings(), as_field=isinstance(self, Field)), self.__R
+        if self in _CommRings:
+            return QuotientFunctor(self.__I, names=names, domain=_CommRings,
+                                   codomain=_CommRings,
+                                   as_field=isinstance(self, Field)), self.__R
         else:
-            return QuotientFunctor(self.__I, names=names, as_field=isinstance(self, Field)), self.__R
+            return QuotientFunctor(self.__I, names=names,
+                                   as_field=isinstance(self, Field)), self.__R
 
     def _repr_(self):
         """
@@ -1290,6 +1289,7 @@ def term_order(self):
         """
         return self.__R.term_order()
 
+
 class QuotientRing_generic(QuotientRing_nc, ring.CommutativeRing):
     r"""
     Creates a quotient ring of a *commutative* ring `R` by the ideal `I`.
@@ -1316,13 +1316,14 @@ def __init__(self, R, I, names, category=None):
 
         TESTS::
 
-            sage: isinstance(ZZ.quo(2), sage.rings.ring.CommutativeRing)  # indirect doctest
+            sage: ZZ.quo(2) in Rings().Commutative()  # indirect doctest
             True
         """
-        if not isinstance(R, ring.CommutativeRing):
+        if R not in _CommRings:
             raise TypeError("This class is for quotients of commutative rings only.\n    For non-commutative rings, use <sage.rings.quotient_ring.QuotientRing_nc>")
         if not self._is_category_initialized():
-            category = check_default_category(_CommutativeRingsQuotients,category)
+            category = check_default_category(_CommutativeRingsQuotients,
+                                              category)
         QuotientRing_nc.__init__(self, R, I, names, category=category)
 
     def _macaulay2_init_(self, macaulay2=None):