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

Hide GAP finiteness warnings during isomorphism check #38956

Merged
merged 2 commits into from
Dec 8, 2024

Conversation

orlitzky
Copy link
Contributor

To fix #38889 and others like it, we hide the "#I Forcing finiteness test warning that can be printed by GAP if one of the groups is not (yet) known to be finite.

…ecks

GAP emits an "InfoWarning" message when it is checking for isomorphism
between two groups that are not known to be finite. This message is
only shown once, however, because after it checks for finiteness, it
knows. We have to keep this in mind during tests. For example,

  File "src/sage/groups/finitely_presented_named.py", line 485, in
  sage.groups.finitely_presented_named.AlternatingPresentation
  Failed example:
      A1.is_isomorphic(A2), A1.order()
  Expected:
      (True, 1)
  Got:
      #I  Forcing finiteness test
      (True, 1)

These warnings are "normal" and there's nothing for the user to do.
This commit hides them for the duration of the isomorphism check.

Fixes sagemath#38889
The warning "#I  Forcing finiteness test" should no longer be emitted
when checking for group isomorphism, so we remove it from the expected
output in a few places.

Fixes sagemath#38889
Copy link

Documentation preview for this PR (built with commit 060f115; changes) is ready! 🎉
This preview will update shortly after each push to this PR.

Copy link
Contributor

@dcoudert dcoudert left a comment

Choose a reason for hiding this comment

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

LGTM.

@orlitzky
Copy link
Contributor Author

orlitzky commented Nov 17, 2024 via email

@tobiasdiez tobiasdiez added the p: CI Fix merged before running CI tests label Nov 18, 2024
vbraun pushed a commit to vbraun/sage that referenced this pull request Dec 6, 2024
    
To fix sagemath#38889 and others like it,
we hide the `"#I  Forcing finiteness test` warning that can be printed
by GAP if one of the groups is not (yet) known to be finite.
    
URL: sagemath#38956
Reported by: Michael Orlitzky
Reviewer(s): David Coudert
vbraun pushed a commit to vbraun/sage that referenced this pull request Dec 8, 2024
    
To fix sagemath#38889 and others like it,
we hide the `"#I  Forcing finiteness test` warning that can be printed
by GAP if one of the groups is not (yet) known to be finite.
    
URL: sagemath#38956
Reported by: Michael Orlitzky
Reviewer(s): David Coudert
@vbraun vbraun merged commit 4434cfd into sagemath:develop Dec 8, 2024
23 of 24 checks passed
@user202729
Copy link
Contributor

Has been a while, but I wonder if hiding the warning is the best idea. After all, warnings are displayed for a reason.

The original motivation is just to make the doctests pass. For that Sage has a custom SageOutputChecker, where you can implement do_fixup().

@orlitzky
Copy link
Contributor Author

Has been a while, but I wonder if hiding the warning is the best idea. After all, warnings are displayed for a reason.

The isomorphism check only works on finite groups, so my thought process was, if it has to check that the groups are finite... okay? Is there any action I should take? If not, it's just noise. Printing unstructured GAP messages to the console also isn't great from the sage UI perspective since there's no easy way to hide them in your own program like there would be with e.g. a python warning.

For power users and developers, the warnings are still available if you really want to see them: just use G.gap().IsomorphismGroups(H.gap()) instead of the sage method.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
p: CI Fix merged before running CI tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Random test failures in finitely_presented_named
5 participants