Trac #13441: refactor gcd

Currently, some classes define a {{{_gcd}}} method which is called by a
{{{gcd}}} method of euclidean domain elements. Most elements already
define {{{gcd}}} directly, and I see no real use in having this method
in between.

The attached patch renames all {{{_gcd}}} methods to {{{gcd}}} and also
streamlines the use of {{{@coerce_binop}}} on them.

This is similar to #13628.

URL: http://trac.sagemath.org/13441
Reported by: saraedum
Ticket author(s): Julian Rueth
Reviewer(s): Niles Johnson, Travis Scrimshaw

src/doc/en/thematic_tutorials/tutorial-objects-and-classes.rst

@@ -297,11 +297,12 @@ Some particular actions modify the data structure of ``el``::
         <type 'sage.rings.integer.Integer'>
         sage: e.__dict__
         dict_proxy({'__module__': 'sage.categories.euclidean_domains',
-        '_reduction': (<built-in function getattr>, (Category of
-        euclidean domains, 'element_class')), '__doc__': None,
+        '__doc__': None, '_reduction': (<built-in function getattr>, (Category
+        of euclidean domains, 'element_class')), 'gcd':
+        <sage.structure.element.NamedBinopMethod object at 0x...>,
         '_sage_src_lines_': <staticmethod object at 0x...>})
         sage: e.__dict__.keys()
-        ['__module__', '_reduction', '__doc__', '_sage_src_lines_']
+        ['__module__', '__doc__', '_reduction', 'gcd', '_sage_src_lines_']
 
         sage: id4 = SymmetricGroup(4).one()
         sage: type(id4)

src/sage/categories/euclidean_domains.py

@@ -12,6 +12,7 @@
 from sage.categories.category_singleton import Category_singleton
 from sage.categories.principal_ideal_domains import PrincipalIdealDomains
 from sage.misc.cachefunc import cached_method
+from sage.structure.element import coerce_binop
 
 class EuclideanDomains(Category_singleton):
     """
@@ -54,4 +55,34 @@ def is_euclidean_domain(self):
             return True
 
     class ElementMethods:
-        pass
+        @coerce_binop
+        def gcd(self, other):
+            """
+            Return the greatest common divisor of this element and ``other``.
+
+            INPUT:
+
+            - ``other`` -- an element in the same ring as ``self``
+
+            ALGORITHM:
+
+            Algorithm 3.2.1 in [Coh1996]_.
+
+            REFERENCES:
+
+            .. [Coh1996] Henri Cohen. *A Course in Computational Algebraic
+               Number Theory*. Springer, 1996.
+
+            EXAMPLES::
+
+                sage: EuclideanDomains().ElementMethods().gcd(6,4)
+                2
+            """
+            A = self
+            B = other
+            while not B.is_zero():
+                Q, R = A.quo_rem(B)
+                A = B
+                B = R
+            return A
+

src/sage/rings/polynomial/polynomial_element_generic.py

@@ -632,11 +632,25 @@ def quo_rem(self, other):
             R = R[:R.degree()] - (aaa*B[:B.degree()]).shift(diff_deg)
         return (Q, R)
 
-    def _gcd(self, other):
+    @coerce_binop
+    def gcd(self, other):
         """
-        Return the GCD of self and other, as a monic polynomial.
+        Return the greatest common divisor of this polynomial and ``other``, as
+        a monic polynomial.
+
+        INPUT:
+
+        - ``other`` -- a polynomial defined over the same ring as ``self``
+
+        EXAMPLES::
+
+            sage: R.<x> = QQbar[]
+            sage: (2*x).gcd(2*x^2)
+            x
+
         """
-        g = EuclideanDomainElement._gcd(self, other)
+        from sage.categories.euclidean_domains import EuclideanDomains
+        g = EuclideanDomains().ElementMethods().gcd(self, other)
         c = g.leading_coefficient()
         if c.is_unit():
             return (1/c)*g

src/sage/rings/polynomial/polynomial_modn_dense_ntl.pyx

@@ -1750,16 +1750,22 @@ cdef class Polynomial_dense_mod_p(Polynomial_dense_mod_n):
 
     @coerce_binop
     def gcd(self, right):
-        return self._gcd(right)
-
-    def _gcd(self, right):
         """
-        Return the GCD of self and other, as a monic polynomial.
+        Return the greatest common divisor of this polynomial and ``other``, as
+        a monic polynomial.
+
+        INPUT:
+
+        - ``other`` -- a polynomial defined over the same ring as ``self``
+
+        EXAMPLES::
+
+            sage: R.<x> = PolynomialRing(GF(3),implementation="NTL")
+            sage: f,g = x + 2, x^2 - 1
+            sage: f.gcd(g)
+            x + 2
+
         """
-        if not isinstance(right, Polynomial_dense_mod_p):
-            right = self.parent()(right)
-        elif self.parent() != right.parent():
-            raise TypeError
         g = self.ntl_ZZ_pX().gcd(right.ntl_ZZ_pX())
         return self.parent()(g, construct=True)
 

src/sage/rings/rational.pyx

@@ -902,28 +902,6 @@ cdef class Rational(sage.structure.element.FieldElement):
         from sage.rings.arith import gcd, lcm
         return gcd(nums) / lcm(denoms)
 
-#    def gcd_rational(self, other, **kwds):
-#        """
-#        Return a gcd of the rational numbers self and other.
-#
-#        If self = other = 0, this is by convention 0.  In all other
-#        cases it can (mathematically) be any nonzero rational number,
-#        but for simplicity we choose to always return 1.
-#
-#        EXAMPLES::
-#
-#            sage: (1/3).gcd_rational(2/1)
-#            1
-#            sage: (1/1).gcd_rational(0/1)
-#            1
-#            sage: (0/1).gcd_rational(0/1)
-#            0
-#        """
-#        if self == 0 and other == 0:
-#            return Rational(0)
-#        else:
-#            return Rational(1)
-
     def valuation(self, p):
         r"""
         Return the power of ``p`` in the factorization of self.
@@ -3145,33 +3123,6 @@ cdef class Rational(sage.structure.element.FieldElement):
             return Rational(0)
         return Rational(1)
 
-    def _gcd(self, Rational other):
-        """
-        Returns the least common multiple, in the rational numbers, of ``self``
-        and ``other``. This function returns either 0 or 1 (as a rational
-        number).
-
-        INPUT:
-
-        -  ``other`` - Rational
-
-        OUTPUT:
-
-        -  ``Rational`` - 0 or 1
-
-        EXAMPLES::
-
-            sage: (2/3)._gcd(3/5)
-            1
-            sage: (0/1)._gcd(0/1)
-            0
-        """
-        if mpz_cmp_si(mpq_numref(self.value), 0) == 0 and \
-               mpz_cmp_si(mpq_numref(other.value), 0) == 0:
-            return Rational(0)
-        return Rational(1)
-
-
     def additive_order(self):
         """
         Return the additive order of ``self``.

src/sage/structure/element.pyx

@@ -2774,14 +2774,6 @@ cdef class PrincipalIdealDomainElement(DedekindDomainElement):
             return coercion_model.bin_op(self, right, lcm)
         return self._lcm(right)
 
-    def gcd(self, right):
-        """
-        Returns the gcd of self and right, or 0 if both are 0.
-        """
-        if not PY_TYPE_CHECK(right, Element) or not ((<Element>right)._parent is self._parent):
-            return coercion_model.bin_op(self, right, gcd)
-        return self._gcd(right)
-
     def xgcd(self, right):
         r"""
         Return the extended gcd of self and other, i.e., elements `r, s, t` such that
@@ -2816,20 +2808,6 @@ cdef class EuclideanDomainElement(PrincipalIdealDomainElement):
     def degree(self):
         raise NotImplementedError
 
-    def _gcd(self, other):
-        """
-        Return the greatest common divisor of self and other.
-
-        Algorithm 3.2.1 in Cohen, GTM 138.
-        """
-        A = self
-        B = other
-        while not B.is_zero():
-            Q, R = A.quo_rem(B)
-            A = B
-            B = R
-        return A
-
     def leading_coefficient(self):
         raise NotImplementedError
 
@@ -2914,15 +2892,6 @@ cdef class FieldElement(CommutativeRingElement):
         """
         return not not self
 
-    def _gcd(self, FieldElement other):
-        """
-        Return the greatest common divisor of self and other.
-        """
-        if self.is_zero() and other.is_zero():
-            return self
-        else:
-            return self._parent(1)
-
     def _lcm(self, FieldElement other):
         """
         Return the least common multiple of self and other.