Moving permutation groups to the new coercion model and style parents.

src/sage/combinat/root_system/reflection_group_complex.py

@@ -890,19 +890,6 @@ def is_crystallographic(self):
         """
         return self.is_real() and all(t.to_matrix().base_ring() is QQ for t in self.simple_reflections())
 
-    def _element_class(self):
-        r"""
-        A temporary workaround for compatibility with Sage's
-        permutation groups.
-
-        TESTS::
-
-            sage: W = ReflectionGroup(23)                               # optional - gap3
-            sage: W._element_class() is W.element_class                 # optional - gap3
-            True
-        """
-        return self.element_class
-
     def number_of_irreducible_components(self):
         r"""
         Return the number of irreducible components of ``self``.

src/sage/combinat/root_system/weyl_group.py

@@ -1319,19 +1319,6 @@ def simple_root_index(self, i):
         """
         return self._index_set_inverse[i]
 
-    def _element_class(self):
-        r"""
-        A temporary workaround for compatibility with Sage's
-        permutation groups.
-
-        TESTS::
-
-            sage: W = WeylGroup(['B',3], implementation="permutation")
-            sage: W._element_class() is W.element_class
-            True
-        """
-        return self.element_class
-
     class Element(RealReflectionGroupElement):
         def _repr_(self):
             """

src/sage/groups/perm_gps/cubegroup.py

@@ -1219,7 +1219,7 @@ def move(self, g):
             sage: RubiksCube().move("R*U") == RubiksCube("R*U")
             True
         """
-        if not isinstance(g, self._group._element_class()):
+        if not isinstance(g, self._group.element_class):
             g = self._group.move(g)[0]
         return RubiksCube(self._state * g, self._history + [g], self.colors)
 

src/sage/groups/perm_gps/permgroup.py

@@ -136,7 +136,7 @@
 from functools import wraps
 
 from sage.misc.randstate import current_randstate
-import sage.groups.old as group
+from sage.groups.group import FiniteGroup
 
 from sage.rings.all import QQ, Integer
 from sage.interfaces.expect import is_ExpectElement
@@ -345,7 +345,7 @@ def PermutationGroup(gens=None, gap_group=None, domain=None, canonicalize=True,
 
 
 @richcmp_method
-class PermutationGroup_generic(group.FiniteGroup):
+class PermutationGroup_generic(FiniteGroup):
     """
     A generic permutation group.
 
@@ -402,8 +402,7 @@ def __init__(self, gens=None, gap_group=None, canonicalize=True, domain=None, ca
             sage: TestSuite(PermutationGroup([(0,1)])).run()
         """
         from sage.categories.permutation_groups import PermutationGroups
-        category = PermutationGroups().FinitelyGenerated().Finite() \
-                   .or_subcategory(category)
+        category = PermutationGroups().FinitelyGenerated().Finite().or_subcategory(category)
         super(PermutationGroup_generic, self).__init__(category=category)
         if (gens is None and gap_group is None):
             raise ValueError("you must specify gens or gap_group")
@@ -438,7 +437,7 @@ def __init__(self, gens=None, gap_group=None, canonicalize=True, domain=None, ca
 
         if not gens:  # length 0
             gens = [()]
-        gens = [self._element_class()(x, self, check=False) for x in gens]
+        gens = [self.element_class(x, self, check=False) for x in gens]
         if canonicalize:
             gens = sorted(set(gens))
         self._gens = gens
@@ -598,22 +597,9 @@ def __richcmp__(self, right, op):
             return rich_to_bool(op, -1)
         return rich_to_bool(op, gapcmp)
 
-    def _element_class(self):
-        r"""
-        Return the class to be used for creating elements of this group. By
-        default this is
-        ``sage.groups.perm_gps.permgroup_element.PermutationGroupElement``, but
-        it may be overridden in derived subclasses (most importantly
-        ``sage.rings.number_field.galois_group.GaloisGroup_v2``).
-
-        EXAMPLES::
+    Element = PermutationGroupElement
 
-            sage: AlternatingGroup(17)._element_class()
-            <type 'sage.groups.perm_gps.permgroup_element.PermutationGroupElement'>
-        """
-        return PermutationGroupElement
-
-    def __call__(self, x, check=True):
+    def _element_constructor_(self, x, check=True):
         """
         Coerce ``x`` into this permutation group.
 
@@ -664,25 +650,47 @@ def __call__(self, x, check=True):
             ...
             TypeError: permutation [(1, 2)] not in Permutation Group with generators [(1,2,3,4)]
         """
-        try:
-            if x.parent() is self:
-                return x
-        except AttributeError:
-            pass
-
         if isinstance(x, integer_types + (Integer,)) and x == 1:
             return self.identity()
 
-        return self._element_class()(x, self, check=check)
+        if isinstance(x, PermutationGroupElement):
+            x_parent = x.parent()
+            if (isinstance(x_parent, PermutationGroup_subgroup)
+                and x_parent._ambient_group is self):
+                return self.element_class(x.cycle_tuples(), self, check=False)
+
+            from sage.groups.perm_gps.permgroup_named import SymmetricGroup
+            compatible_domains = all(point in self._domain_to_gap
+                                     for point in x_parent.domain())
+            if compatible_domains and (isinstance(self, SymmetricGroup)
+                                       or x._gap_() in self._gap_()):
+                return self.element_class(x.cycle_tuples(), self, check=False)
+
+        return self.element_class(x, self, check=check)
 
-    def _coerce_impl(self, x):
+    def _coerce_map_from_(self, G):
         r"""
-        Implicit coercion of ``x`` into ``self``.
+        Return if there is a coercion map from ``G`` into ``self``.
 
         EXAMPLES:
 
-        We illustrate some arithmetic that involves implicit
-        coercion of elements in different permutation groups::
+        We have coercion maps from subgroups::
+
+            sage: G = SymmetricGroup(5)
+            sage: H = G.subgroup([[(1,2,3)], [(4,5)]])
+            sage: G._coerce_map_from_(H)
+            True
+            sage: K = H.subgroup([[(1,2,3)]])
+            sage: H.has_coerce_map_from(K)
+            True
+            sage: G.has_coerce_map_from(K)
+            True
+            sage: G = PermutationGroup([[(1,2)], [(3,4,5)], [(2,3)]])
+            sage: G.has_coerce_map_from(K)
+            True
+
+        We illustrate some arithmetic that involves coercion
+        of elements in different permutation groups::
 
             sage: g1 = PermutationGroupElement([(1,2),(3,4,5)])
             sage: g1.parent()
@@ -697,25 +705,23 @@ def _coerce_impl(self, x):
             sage: g2*g1
             (3,4,5)
 
-        We try to implicitly coerce in a non-permutation, which raises a
-        TypeError::
+        We try to convert in a non-permutation::
 
             sage: G = PermutationGroup([[(1,2,3,4)], [(1,2)]])
-            sage: G._coerce_impl(1)
+            sage: G(2)
             Traceback (most recent call last):
             ...
-            TypeError: no implicit coercion of element into permutation group
-        """
-        from .permgroup_named import SymmetricGroup
-        if isinstance(x, PermutationGroupElement):
-            x_parent = x.parent()
-            if x_parent is self:
-                return x
-
-            compatible_domains = all(point in self._domain_to_gap for point in x_parent.domain())
-            if  compatible_domains and (self.__class__ == SymmetricGroup or x._gap_() in self._gap_()):
-                return self._element_class()(x.cycle_tuples(), self, check=False)
-        raise TypeError("no implicit coercion of element into permutation group")
+            TypeError: 'sage.rings.integer.Integer' object is not iterable
+        """
+        if isinstance(G, PermutationGroup_subgroup):
+            if G._ambient_group is self:
+                return True
+            if self.has_coerce_map_from(G._ambient_group):
+                return self._coerce_map_via([G._ambient_group], G)
+        if isinstance(G, PermutationGroup_generic):
+            if G.is_subgroup(self):
+                return True
+        return super(PermutationGroup_generic, self)._coerce_map_from_(G)
 
     def list(self):
         """
@@ -773,13 +779,14 @@ def __contains__(self, item):
             sage: [('a', 'b')] in G
             True
         """
+        if isinstance(item, integer_types + (Integer,)):
+            return item == 1
         try:
-            item = self(item, check=True)
+            item = self._element_constructor_(item, check=True)
         except Exception:
             return False
         return True
 
-
     def has_element(self, item):
         """
         Returns boolean value of ``item in self`` - however *ignores*
@@ -894,7 +901,7 @@ def gens_small(self):
             True
         """
         gens = self._gap_().SmallGeneratingSet()
-        return [self._element_class()(x, self, check=False) for x in gens]
+        return [self.element_class(x, self, check=False) for x in gens]
 
     def gen(self, i=None):
         r"""
@@ -966,7 +973,7 @@ def identity(self):
             sage: S.identity()
             ()
         """
-        return self._element_class()([], self, check=True)
+        return self.element_class([], self, check=True)
 
     def exponent(self):
         r"""
@@ -1133,7 +1140,7 @@ def representative_action(self,x,y):
         """
         ans = self._gap_().RepresentativeAction(self._domain_to_gap[x],
                                                 self._domain_to_gap[y])
-        return self._element_class()(ans, self, check=False)
+        return self.element_class(ans, self, check=False)
 
     @cached_method
     def orbits(self):
@@ -1954,7 +1961,7 @@ def conjugacy_classes(self):
         cl = self._gap_().ConjugacyClasses()
         n = Integer(cl.Length())
         L = gap("List([1..Length(%s)], i->Representative(%s[i]))"%(cl.name(),  cl.name()))
-        return [ConjugacyClassGAP(self,self._element_class()(L[i], self, check=False)) for i in range(1,n+1)]
+        return [ConjugacyClassGAP(self,self.element_class(L[i], self, check=False)) for i in range(1,n+1)]
 
     def conjugate(self, g):
         r"""
@@ -4278,7 +4285,7 @@ def __init__(self, ambient, gens=None, gap_group=None, domain=None,
             TypeError: each generator must be in the ambient group
         """
         if not isinstance(ambient, PermutationGroup_generic):
-            raise TypeError("ambient (=%s) must be perm group."%ambient)
+            raise TypeError("ambient (=%s) must be perm group"%ambient)
         if domain is None:
             domain = ambient.domain()
         if category is None:
@@ -4288,9 +4295,12 @@ def __init__(self, ambient, gens=None, gap_group=None, domain=None,
 
         self._ambient_group = ambient
         if check:
-            for g in self.gens():
-                if g not in ambient:
-                    raise TypeError("each generator must be in the ambient group")
+            from sage.groups.perm_gps.permgroup_named import SymmetricGroup
+            if not isinstance(ambient, SymmetricGroup):
+                ambient_gap_group = ambient._gap_()
+                for g in self.gens():
+                    if g._gap_() not in ambient_gap_group:
+                        raise TypeError("each generator must be in the ambient group")
 
     def __richcmp__(self, other, op):
         r"""

src/sage/groups/perm_gps/permgroup_element.pyx

@@ -129,9 +129,9 @@ def make_permgroup_element_v2(G, x, domain):
     # G._domain_to_gap be set.
     G._domain = domain
     G._deg = len(domain)
-    G._domain_to_gap = dict([(key, i+1) for i, key in enumerate(domain)])
-    G._domain_from_gap = dict([(i+1, key) for i, key in enumerate(domain)])
-    return G(x, check=False)
+    G._domain_to_gap = {key: i+1 for i, key in enumerate(domain)}
+    G._domain_from_gap = {i+1: key for i, key in enumerate(domain)}
+    return G.element_class(x, G, check=False)
 
 
 def is_PermutationGroupElement(x):

src/sage/groups/perm_gps/permgroup_named.py

@@ -278,7 +278,7 @@ def __init__(self, domain=None):
         gens = [tuple(self._domain)]
         if len(self._domain) > 2:
             gens.append(tuple(self._domain[:2]))
-        self._gens = [self._element_class()(g, self, check=False)
+        self._gens = [self.element_class(g, self, check=False)
                       for g in gens]
 
     def _gap_init_(self, gap=None):
@@ -591,9 +591,9 @@ def algebra(self, base_ring, category=None):
             sage: A = S3.algebra(QQ); A
             Symmetric group algebra of order 3 over Rational Field
             sage: a = S3.an_element(); a
-            (1,2,3)
+            (2,3)
             sage: A(a)
-            (1,2,3)
+            (2,3)
 
         We illustrate the choice of the category::
 
@@ -613,9 +613,9 @@ def algebra(self, base_ring, category=None):
             sage: S.algebra(QQ)
             Algebra of Symmetric group of order 3! as a permutation group over Rational Field
             sage: a = S.an_element(); a
-            (2,3,5)
+            (3,5)
             sage: S.algebra(QQ)(a)
-            (2,3,5)
+            (3,5)
         """
         from sage.combinat.symmetric_group_algebra import SymmetricGroupAlgebra
         domain = self.domain()
@@ -624,16 +624,7 @@ def algebra(self, base_ring, category=None):
         else:
             return super(SymmetricGroup, self).algebra(base_ring)
 
-    def _element_class(self):
-        r"""
-        Return the class to be used for creating elements of this group.
-
-        EXAMPLES::
-
-            sage: SymmetricGroup(17)._element_class()
-            <type 'sage.groups.perm_gps.permgroup_element.SymmetricGroupElement'>
-        """
-        return SymmetricGroupElement
+    Element = SymmetricGroupElement
 
 class AlternatingGroup(PermutationGroup_symalt):
     def __init__(self, domain=None):