Directly convert PermutationGroup element into sized Permutation

[RuchitJagodara]

This patch fixes #37284 by converting the input into a Permutation class object if
it is the object of another class.


📝 Checklist

• The title is concise, informative, and self-explanatory.
• The description explains in detail what this PR is about.
• I have linked a relevant issue or discussion.
• I have created tests covering the changes.

⌛ Dependencies

[RuchitJagodara]

Does anybody know any better way so that we can avoid try except block here ?

[tscrim]

Does anybody know any better way so that we can avoid try except block here ?

See my comment. Checking for specific types is usually better than checking for not a specific type when trying to apply methods.

[grhkm21]

Oops, learned something myself. I will keep that in mind too :)

[RuchitJagodara]

I have done all necessary changes that you suggested, thank you for your suggestions ! : )

[tscrim]

Thank you. However, your test is not good because that was working before from the coercion map defined using P5._from_permutation_group_element. It would probably be better to go through that method here as well. (Yes, it does the same thing with going back through the _element_constructor_, but it might do something smarter/faster in the future. It also makes the code easier to debug should a problem arise later.) Sorry I missed that before.

[mantepse]

Is #37318 related?

[tscrim]

Is #37318 related?

From a quick look, doesn’t seem related.

[RuchitJagodara]

Then, should I change _coerce_map_from function and redirect the flow to the _from_permutation_group_element whenever G is a PermutationGroup, but I don't know what to include in isinstance function to check wether the group is permutation group or not ?

sage/src/sage/combinat/permutation.py

Lines 7210 to 7211 in f444d32

	if isinstance(G, SymmetricGroup):
	D = G.domain()

[tscrim]

The test is not that good (it is better to not depend on the ordering of the group, and a slightly more general group on, say, 4 letters would be better), but it is sufficient. Hence, we can merge this as-is, but it would be nice to have a slightly better test.

[RuchitJagodara]

Should I change the test? (Just confirming because @tscrim approved the changes but there is still a 'needs work' label on this PR.)

[tscrim]

@RuchitJagodara I’ve left the decision up to you, but it would be better to have a more generic test.

[RuchitJagodara]

I have updated the test, @tscrim can you please review that ?

[tscrim]

That is essentially the same test. The point is to have a more interesting group than something (trivially) isomorphic to the symmetric group. So something like the group generated by [(1,2)(2,3), (4,5)].

[RuchitJagodara]

Sorry, because of my mid-term exams, I was not able to make these changes earlier. But I have changed the test and it is not dependent on ordering of the group, and is not trivial, also.

[RuchitJagodara]

That is essentially the same test. The point is to have a more interesting group than something (trivially) isomorphic to the symmetric group. So something like the group generated by [(1,2)(2,3), (4,5)].

When I try to create a permutation group using the generators [(1,2)(2,3), (4,5)], it gives me a ValueError, saying 'invalid data to initialize a permutation.' I am a bit confused as to why that error is occurring.

[tscrim]

That is essentially the same test. The point is to have a more interesting group than something (trivially) isomorphic to the symmetric group. So something like the group generated by [(1,2)(2,3), (4,5)].

When I try to create a permutation group using the generators [(1,2)(2,3), (4,5)], it gives me a ValueError, saying 'invalid data to initialize a permutation.' I am a bit confused as to why that error is occurring.

Sorry, there was a typo on my part. I meant

sage: G = PermutationGroup([[(1,2),(3,4)], [(4,5)]])
sage: G.isomorphism_to(DihedralGroup(6))
Permutation group morphism:
  From: Permutation Group with generators [(4,5), (1,2)(3,4)]
  To:   Dihedral group of order 12 as a permutation group
  Defn: [(4,5), (1,2)(3,4)] -> [(1,2)(3,6)(4,5), (1,3)(4,6)]

Although you could interpret it as

sage: G = PermutationGroup([[(1,2,3)], [(4,5)]])  # = Z3 x Z2

[tscrim]

Anyways, the test is better now. Thank you. Just a few details left.

[RuchitJagodara]

I have updated the test. Thank you for your suggestions.

[tscrim]

Thank you. LGTM now.

[github-actions]

Documentation preview for this PR (built with commit 54747b6; changes) is ready! 🎉