Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

make gens and orbits of PermutationGroup immutable #33824

Closed
mantepse opened this issue May 9, 2022 · 19 comments
Closed

make gens and orbits of PermutationGroup immutable #33824

mantepse opened this issue May 9, 2022 · 19 comments

Comments

@mantepse
Copy link
Contributor

mantepse commented May 9, 2022

We don't want the following to happen:

sage: G = PermutationGroup([ [(3,4)], [(1,3)] ])
sage: O = G.orbits(); O
[[1, 3, 4], [2]]
sage: O[0] = 1
sage: G.orbits()
[1, [2]]

sage: G = PermutationGroup([[2,1]]); G
Permutation Group with generators [(1,2)]
sage: g = G.gens()
sage: g[0] = 1
sage: G.gens()
[1]

Therefore, we change the return type of gens and orbits to be a tuple of tuples. We keep the _repr_ of PermutationGroup_generic, because the output is more compact.

Note that gens returns a tuple for all other groups I checked.

CC: @tscrim

Component: group theory

Author: Martin Rubey

Branch/Commit: 90005e7

Reviewer: Travis Scrimshaw

Issue created by migration from https://trac.sagemath.org/ticket/33824

@mantepse mantepse added this to the sage-9.7 milestone May 9, 2022
@mantepse
Copy link
Contributor Author

mantepse commented May 9, 2022

@mantepse

This comment has been minimized.

@mantepse
Copy link
Contributor Author

mantepse commented May 9, 2022

New commits:

3f65072make PermutationGroup_generic.orbits return a tuple of tuples, for immutability

@mantepse
Copy link
Contributor Author

mantepse commented May 9, 2022

Commit: 3f65072

@mantepse
Copy link
Contributor Author

mantepse commented May 9, 2022

Author: Martin Rubey

@mantepse
Copy link
Contributor Author

mantepse commented May 9, 2022

comment:3

Here is another problem, of the same kind:

sage: G = PermutationGroup([[2,1]]); G
Permutation Group with generators [(1,2)]
sage: g = G.gens()
sage: g[0] = 1
sage: G.gens()
[1]

I checked a few examples of groups, the all return a tuple.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 9, 2022

Changed commit from 3f65072 to da2c2e5

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 9, 2022

Branch pushed to git repo; I updated commit sha1. New commits:

da2c2e5make PermutationGroup_generic.gens return a tuple, to ensure immutability

@mantepse

This comment has been minimized.

@mantepse mantepse changed the title make orbits of PermutationGroup immutable make gens and orbits of PermutationGroup immutable May 9, 2022
@mantepse

This comment has been minimized.

@tscrim
Copy link
Collaborator

tscrim commented May 10, 2022

comment:7

A while-we-are-at-it thing would be to replace the $ with a back tick `. If you decide to do this or not, you can set a positive review on my behalf once the patchbot is green.

@tscrim
Copy link
Collaborator

tscrim commented May 10, 2022

Reviewer: Travis Scrimshaw

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 10, 2022

Branch pushed to git repo; I updated commit sha1. New commits:

d5adcb0replace $...$ with `...`

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 10, 2022

Changed commit from da2c2e5 to d5adcb0

@mantepse
Copy link
Contributor Author

comment:9

Done, I also looked at the doc which comes out fine.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 10, 2022

Changed commit from d5adcb0 to 90005e7

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 10, 2022

Branch pushed to git repo; I updated commit sha1. New commits:

90005e7remove unused import

@tscrim
Copy link
Collaborator

tscrim commented May 10, 2022

comment:11

Thank you. LGTM.

@vbraun
Copy link
Member

vbraun commented May 22, 2022

Changed branch from u/mantepse/make_orbits_of_permutationgroup_immutable to 90005e7

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants