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

Fix WreathProductElementList to not modify its list argument #5801

Merged
merged 5 commits into from
Sep 23, 2024

Conversation

FriedrichRober
Copy link
Contributor

Replace StructuralCopy with ShallowCopy in WreathProductElementListNC.

Otherwise an immutable list representation from the input gets replaced with the wreath product element even outside of the scope of WreathProductElementList.

Setup:

gap> K := FreeGroup("x", "y");;
gap> AssignGeneratorVariables(K);;
gap> H := SymmetricGroup(3);;
gap> W := WreathProduct(K, H);
<group with 4 generators>
gap> l := [x*y, x, y, (1,2,3)];
[ x*y, x, y, (1,2,3) ]
gap> MakeImmutable(l);;
gap> w := WreathProductElementList(W, l);
WreathProductElement(x*y,x,y,(1,2,3))

Current behaviour: list l gets overwritten by output w.

gap> l;
WreathProductElement(x*y,x,y,(1,2,3))

Expected behaviur: list l should be unchanged

gap> l;
[ x*y, x, y, (1,2,3) ]

Text for release notes

Fix a bug in WreathProductElementList for generic wreath products, which replaced an immutable list from the input with the constructed wreath product element.

Replace `StructuralCopy` with `ShallowCopy`.
Otherwise the list representation from the input gets replaced with the wreath product element outside of the scope.
@FriedrichRober FriedrichRober added the kind: bug Issues describing general bugs, and PRs fixing them label Sep 19, 2024
@james-d-mitchell
Copy link
Contributor

Perhaps add a test?

@fingolfin fingolfin changed the title FIX: WreathProductElementList for immutable lists Fix WreathProductElementList to not modify its list argument Sep 19, 2024
Copy link
Member

@fingolfin fingolfin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, fix makes sense. But it would indeed be nice you could add your example as a test case.

@fingolfin fingolfin added topic: library backport-to-4.13 release notes: use title For PRs: the title of this PR is suitable for direct use in the release notes labels Sep 19, 2024
Add a test file for the bugfix
@FriedrichRober
Copy link
Contributor Author

@fingolfin, I added a test file. You can squash and merge if everything looks fine.

@fingolfin fingolfin merged commit 2a7cf16 into master Sep 23, 2024
37 checks passed
@fingolfin fingolfin deleted the FIX-WreathProductElementList branch September 23, 2024 08:48
@fingolfin
Copy link
Member

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-to-4.13 kind: bug Issues describing general bugs, and PRs fixing them release notes: use title For PRs: the title of this PR is suitable for direct use in the release notes topic: library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants