sagemathgh-37449: formal deprecation of old group class

    
trying to finally get rid of the auld group class

### 📝 Checklist

- [x] The title is concise, informative, and self-explanatory.
- [x] The description explains in detail what this PR is about.
- [x] I have created tests covering the changes.
- [x] I have updated the documentation accordingly.
    
URL: sagemath#37449
Reported by: Frédéric Chapoton
Reviewer(s): Frédéric Chapoton, Matthias Köppe

src/sage/categories/action.pyx

@@ -381,10 +381,10 @@ cdef class InverseAction(Action):
     def __init__(self, Action action):
         G = action.G
         try:
-            from sage.groups.group import is_Group
+            from sage.groups.group import Group
             # We must be in the case that parent(~a) == parent(a)
             # so we can invert in _call_ code below.
-            if (is_Group(G) and G.is_multiplicative()) or G.is_field():
+            if (isinstance(G, Group) and G.is_multiplicative()) or G.is_field():
                 Action.__init__(self, G, action.underlying_set(), action._is_left)
                 self._action = action
                 return

src/sage/groups/additive_abelian/additive_abelian_group.py

@@ -6,9 +6,9 @@
 major differences are in the way elements are printed.
 """
 
-from sage.groups.old import AbelianGroup
-from sage.modules.fg_pid.fgp_module import FGP_Module_class
+from sage.categories.commutative_additive_groups import CommutativeAdditiveGroups
 from sage.modules.fg_pid.fgp_element import FGP_Element
+from sage.modules.fg_pid.fgp_module import FGP_Module_class
 from sage.rings.integer_ring import ZZ
 
 
@@ -173,12 +173,12 @@ def _hermite_lift(self):
         for i in range(H.nrows()):
             if i in pivot_rows:
                 j = pivots[i]
-                N = H[i,j]
+                N = H[i, j]
                 a = (y[j] - (y[j] % N)) // N
-                y = y - a*H.row(i)
+                y = y - a * H.row(i)
         return y
 
-    def _repr_(self):
+    def _repr_(self) -> str:
         r"""
         String representation. This uses a canonical lifting of elements of
         this group (represented as a quotient `G/H` of free abelian groups) to
@@ -201,7 +201,7 @@ def _repr_(self):
 # since we want to inherit things like __hash__ from there rather than the
 # hyper-generic implementation for abstract abelian groups.
 
-class AdditiveAbelianGroup_class(FGP_Module_class, AbelianGroup):
+class AdditiveAbelianGroup_class(FGP_Module_class):
     r"""
     An additive abelian group, implemented using the `\ZZ`-module machinery.
 
@@ -223,6 +223,10 @@ def __init__(self, cover, relations):
             Additive abelian group isomorphic to Z
             sage: G == loads(dumps(G))
             True
+            sage: G.category()
+            Category of modules over Integer Ring
+            sage: G in CommutativeAdditiveGroups()
+            True
         """
         FGP_Module_class.__init__(self, cover, relations)
 

src/sage/groups/group.pyx

@@ -16,9 +16,9 @@ Base class for groups
 #
 #                  https://www.gnu.org/licenses/
 # ****************************************************************************
-
-from sage.structure.parent cimport Parent
+from sage.misc.superseded import deprecation
 from sage.rings.infinity import infinity
+from sage.structure.parent cimport Parent
 
 
 def is_Group(x):
@@ -38,12 +38,14 @@ def is_Group(x):
         sage: F.<a,b> = FreeGroup()                                                     # needs sage.groups
         sage: from sage.groups.group import is_Group
         sage: is_Group(F)                                                               # needs sage.groups
+        doctest:warning...DeprecationWarning: use instead G in Groups()
+        See https://github.com/sagemath/sage/issues/37449 for details.
         True
         sage: is_Group("a string")
         False
     """
-    from sage.groups.old import Group as OldGroup
-    return isinstance(x, (Group, OldGroup))
+    deprecation(37449, 'use instead G in Groups()')
+    return isinstance(x, Group)
 
 
 cdef class Group(Parent):

src/sage/groups/old.pyx

@@ -1,5 +1,5 @@
 """
-Base class for groups
+Deprecated base class for groups
 """
 
 # ****************************************************************************
@@ -17,11 +17,11 @@ Base class for groups
 #                  https://www.gnu.org/licenses/
 # ****************************************************************************
 
-doc="""
-Base class for all groups
+doc = """
+Deprecated base class for all groups
 """
-from sage.rings.infinity import infinity
 import sage.rings.integer_ring
+from sage.misc.superseded import deprecation
 
 
 cdef class Group(sage.structure.parent.Parent):
@@ -35,6 +35,9 @@ cdef class Group(sage.structure.parent.Parent):
 
             sage: from sage.groups.old import Group
             sage: G = Group()
+            doctest:warning...:
+            DeprecationWarning: do not use the old Group class
+            See https://github.com/sagemath/sage/issues/37449 for details.
             sage: G.category()
             Category of groups
             sage: G = Group(category = Groups()) # todo: do the same test with some subcategory of Groups when there will exist one
@@ -54,6 +57,7 @@ cdef class Group(sage.structure.parent.Parent):
             sage: h == hash(G)
             True
         """
+        deprecation(37449, 'do not use the old Group class')
         from sage.categories.basic import Groups
         if category is None:
             category = Groups()
@@ -63,12 +67,6 @@ cdef class Group(sage.structure.parent.Parent):
         sage.structure.parent.Parent.__init__(self,
                 base=sage.rings.integer_ring.ZZ, category=category)
 
-    #def __call__(self, x): # this gets in the way of the coercion mechanism
-    #    """
-    #    Coerce x into this group.
-    #    """
-    #    raise NotImplementedError
-
     def __contains__(self, x):
         r"""
         True if coercion of `x` into self is defined.
@@ -88,13 +86,6 @@ cdef class Group(sage.structure.parent.Parent):
             return False
         return True
 
-#    def category(self):
-#        """
-#        The category of all groups
-#        """
-#        import sage.categories.all
-#        return sage.categories.all.Groups()
-
     def is_abelian(self):
         """
         Return True if this group is abelian.
@@ -142,21 +133,6 @@ cdef class Group(sage.structure.parent.Parent):
         """
         raise NotImplementedError
 
-    def is_finite(self):
-        """
-        Returns True if this group is finite.
-
-        EXAMPLES::
-
-            sage: from sage.groups.old import Group
-            sage: G = Group()
-            sage: G.is_finite()
-            Traceback (most recent call last):
-            ...
-            NotImplementedError
-        """
-        return self.order() != infinity
-
     def is_multiplicative(self):
         r"""
         Returns True if the group operation is given by \* (rather than

src/sage/modular/arithgroup/arithgroup_generic.py

@@ -14,12 +14,12 @@
 #
 ################################################################################
 
-from sage.groups.old import Group
+from sage.groups.group import Group
 from sage.categories.groups import Groups
 from sage.rings.integer_ring import ZZ
 from sage.arith.functions import lcm
 from sage.misc.cachefunc import cached_method
-from copy import copy # for making copies of lists of cusps
+from copy import copy  # for making copies of lists of cusps
 from sage.modular.modsym.p1list import lift_to_sl2z
 from sage.modular.cusps import Cusp
 
@@ -30,7 +30,7 @@
 from .arithgroup_element import ArithmeticSubgroupElement, M2Z as Mat2Z
 
 
-def is_ArithmeticSubgroup(x):
+def is_ArithmeticSubgroup(x) -> bool:
     r"""
     Return ``True`` if ``x`` is of type :class:`ArithmeticSubgroup`.
 
@@ -67,7 +67,7 @@ def __init__(self):
         """
         Group.__init__(self, category=Groups().Infinite())
 
-    def _repr_(self):
+    def _repr_(self) -> str:
         r"""
         Return the string representation of self.