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

Add missing \= method for IsMultiplicativeElement and IsObjWithMemory (the other direction was already present) #4239

Merged

Conversation

FriedrichRober
Copy link
Contributor

@FriedrichRober FriedrichRober commented Feb 2, 2021

Description

\= was implemented for IsObjWithMemory and IsMultiplicativeElement (in this order), but not the other way round.

Text for release notes

Add equality operation for x with objects with memory

Further details

The following is an example of the unexpected behaviour that is fixed by this pull request.

gap> G := GroupWithMemory(SymmetricGroup(5));
Group([ (1,2,3,4,5), (1,2) ])
gap> x := PseudoRandom(G);
<(1,3,2,4,5) with mem>
gap> y := StripMemory(x);
(1,3,2,4,5)
gap> x = y;
true
gap> y = x;
Error, no method found! For debugging hints type ?Recovery from NoMethodFound
Error, no 1st choice method found for `=' on 2 arguments at /usr/local/Cellar/gap/4.11.0/libexec/lib/methsel2.g:249 called from
<function "HANDLE_METHOD_NOT_FOUND">( <arguments> )
 called from read-eval loop at *stdin*:5
type 'quit;' to quit to outer loop
brk> 

Checklist for pull request reviewers

  • proper formatting

If your code contains kernel C code, run clang-format on it; the
simplest way is to use git clang-format, e.g. like this (don't
forget to commit the resulting changes):

git clang-format $(git merge-base HEAD master)
  • usage of relevant labels

    1. either release notes: not needed or release notes: to be added
    2. at least one of the labels bug or enhancement or new feature
    3. for changes meant to be backported to stable-4.X add the backport-to-4.X label
    4. consider adding any of the labels build system, documentation, kernel, library, tests
  • runnable tests

  • lines changed in commits are sufficiently covered by the tests

  • adequate pull request title

  • well formulated text for release notes

  • relevant documentation updates

  • sensible comments in the code

@wilfwilson wilfwilson added kind: bug Issues describing general bugs, and PRs fixing them kind: bug: unexpected error Issues describing bugs in which computation unexpectedly encounters an error, and PRs fixing them release notes: to be added PRs introducing changes that should be (but have not yet been) mentioned in the release notes topic: library labels Feb 3, 2021
@wilfwilson
Copy link
Member

Thanks for the report and the PR. Could you please add your example code to a new test file in tst/testbugfix/? (Following the naming convention there, and ideally without the randomness in the choice of element x).

@FriedrichRober
Copy link
Contributor Author

Ok, the test is added now. I hope that I followed the naming conventions correctly.

Copy link
Member

@wilfwilson wilfwilson left a comment

Choose a reason for hiding this comment

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

Thanks!

@wilfwilson wilfwilson merged commit 940ae67 into gap-system:master Feb 8, 2021
@ThomasBreuer ThomasBreuer self-assigned this Feb 16, 2021
@ThomasBreuer ThomasBreuer added release notes: added PRs introducing changes that have since been mentioned in the release notes and removed release notes: to be added PRs introducing changes that should be (but have not yet been) mentioned in the release notes labels Feb 16, 2021
@ThomasBreuer ThomasBreuer changed the title Add equality operation for x with objects with memory Added missing comparison w.r.t. equality of something and an object with memory (IsObjWithMemory). Feb 16, 2021
@ThomasBreuer ThomasBreuer removed their assignment Feb 16, 2021
@FriedrichRober FriedrichRober deleted the fr/elmWithMemoryEquality branch September 21, 2021 15:01
@fingolfin fingolfin changed the title Added missing comparison w.r.t. equality of something and an object with memory (IsObjWithMemory). Add missing \= method for IsMultiplicativeElement and IsObjWithMemory (the other direction was already present) Aug 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: bug: unexpected error Issues describing bugs in which computation unexpectedly encounters an error, and PRs fixing them kind: bug Issues describing general bugs, and PRs fixing them release notes: added PRs introducing changes that have since been mentioned in the release notes topic: library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants