diff --git a/src/sage/combinat/root_system/reflection_group_complex.py b/src/sage/combinat/root_system/reflection_group_complex.py index 921719932f9..c5ec0b1a458 100644 --- a/src/sage/combinat/root_system/reflection_group_complex.py +++ b/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``. diff --git a/src/sage/combinat/root_system/weyl_group.py b/src/sage/combinat/root_system/weyl_group.py index a3e6a9ba505..6c16f45433e 100644 --- a/src/sage/combinat/root_system/weyl_group.py +++ b/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): """ diff --git a/src/sage/groups/perm_gps/cubegroup.py b/src/sage/groups/perm_gps/cubegroup.py index beef463216b..f449a7bd989 100644 --- a/src/sage/groups/perm_gps/cubegroup.py +++ b/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) diff --git a/src/sage/groups/perm_gps/permgroup.py b/src/sage/groups/perm_gps/permgroup.py index 4c4958371ac..6a1f0e7d54a 100644 --- a/src/sage/groups/perm_gps/permgroup.py +++ b/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() - - """ - 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""" diff --git a/src/sage/groups/perm_gps/permgroup_element.pyx b/src/sage/groups/perm_gps/permgroup_element.pyx index de78fbd8232..3a4ba6d0016 100644 --- a/src/sage/groups/perm_gps/permgroup_element.pyx +++ b/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): diff --git a/src/sage/groups/perm_gps/permgroup_named.py b/src/sage/groups/perm_gps/permgroup_named.py index 2222fc4441a..da1774e96c5 100644 --- a/src/sage/groups/perm_gps/permgroup_named.py +++ b/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() - - """ - return SymmetricGroupElement + Element = SymmetricGroupElement class AlternatingGroup(PermutationGroup_symalt): def __init__(self, domain=None): diff --git a/src/sage/rings/number_field/galois_group.py b/src/sage/rings/number_field/galois_group.py index 691ac17e551..3a92786644b 100644 --- a/src/sage/rings/number_field/galois_group.py +++ b/src/sage/rings/number_field/galois_group.py @@ -167,7 +167,6 @@ def number_field(self): class GaloisGroup_v2(PermutationGroup_generic): - r""" The Galois group of an (absolute) number field. @@ -198,6 +197,20 @@ def __init__(self, number_field, names=None): TypeError: You must specify the name of the generator. sage: NumberField(x^3 - 2, 'b').galois_group(names="c") Galois group of Galois closure in c of Number Field in b with defining polynomial x^3 - 2 + + TESTS:: + + sage: F. = CyclotomicField(7) + sage: G = F.galois_group() + + We test that a method inherited from PermutationGroup_generic returns + the right type of element (see :trac:`133`):: + + sage: phi = G.random_element() + sage: type(phi) is G.element_class + True + sage: phi(z) # random + z^3 """ self._number_field = number_field @@ -219,29 +232,6 @@ def __init__(self, number_field, names=None): # PARI computes all the elements of self anyway, so we might as well store them self._elts = sorted([self(x, check=False) for x in g[5]]) - def _element_class(self): - r""" - Return the class to be used for creating elements of this group, which - is GaloisGroupElement. - - EXAMPLES:: - - sage: F. = CyclotomicField(7) - sage: G = F.galois_group() - sage: G._element_class() - - - We test that a method inherited from PermutationGroup_generic returns - the right type of element (see :trac:`133`):: - - sage: phi = G.random_element() - sage: type(phi) - - sage: phi(z) # random - z^3 - """ - return GaloisGroupElement - def __call__(self, x, check=True): r""" Create an element of self from x. Here x had better be one of: -- the integer 1, denoting the identity of G @@ -272,7 +262,7 @@ def __call__(self, x, check=True): if len(l) != 1: raise ArithmeticError return l[0] - return GaloisGroupElement(x, parent=self, check=check) + return self.element_class(x, parent=self, check=check) def is_galois(self): r""" @@ -788,6 +778,7 @@ def ramification_degree(self, P): w = [(self(g) - g).valuation(P) for g in gens] return min(w) +GaloisGroup_v2.Element = GaloisGroupElement # For unpickling purposes we rebind GaloisGroup as GaloisGroup_v1.