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

use early methods for tag based operations #5396

Merged
merged 1 commit into from
Mar 13, 2023

Conversation

ThomasBreuer
Copy link
Contributor

This is a follow-up of #5344, according to a comment in #4398.

This is a follow-up of gap-system#5344, according to a comment in gap-system#4398.
@ThomasBreuer ThomasBreuer added kind: enhancement Label for issues suggesting enhancements; and for pull requests implementing enhancements topic: library release notes: not needed PRs introducing changes that are wholly irrelevant to the release notes labels Mar 8, 2023
@fingolfin
Copy link
Member

The documentation test fails in the line

d1:= DiagonalMatrix( GF(9), [ 1, 2 ] * Z(3)^0 );

Both @ThomasBreuer and me had trouble reproducing this, so I went and did exactly what the CI does for testing the manuals (i.e. following the steps in dev/ci.sh for the testmanuals test suite...

... and the problem seems to be caused by saving and restoring workspaces:

$ ./gap -b -A
gap> SaveWorkspace("testmanuals.wsp");
true
gap> QUIT;
$ ./gap -b -L testmanuals.wsp
gap> d1:= DiagonalMatrix( GF(9), [ 1, 2 ] * Z(3)^0 );
Error, no method found! For debugging hints type ?Recovery from NoMethodFound
Error, no 1st choice method found for `DiagonalMatrix' on 3 arguments at /Users/mhorn/Projekte/GAP/gap/lib/methsel2.g:250 called from
DiagonalMatrix( DefaultMatrixRepForBaseDomain( R ), R, vector

I've not yet dug deeper from here, I first need some sleep.

@ChrisJefferson
Copy link
Contributor

The workspace issue is fixed (hopefully!) in #5400

@fingolfin fingolfin closed this Mar 9, 2023
@fingolfin fingolfin reopened this Mar 9, 2023
@ThomasBreuer
Copy link
Contributor Author

@ChrisJefferson Thanks a lot for the fix.
As far as I understand, the manual tests are currently the only place where GAP workspaces are exercised.
Would it make sense to add a test that first creates a workspace and then runs the tests from testinstall and teststandard with this workspace?

@fingolfin
Copy link
Member

@ThomasBreuer Yes that would make sense.

@fingolfin fingolfin merged commit 361953a into gap-system:master Mar 13, 2023
@ThomasBreuer ThomasBreuer deleted the TB_tag_based_early_method branch March 13, 2023 13:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: enhancement Label for issues suggesting enhancements; and for pull requests implementing enhancements release notes: not needed PRs introducing changes that are wholly irrelevant to the release notes topic: library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants