Use AutoDoc to extract examples

[fingolfin]

This contains PRs #164, #165, #166 and also will still produce errors. I recommend merging the other three PRs first, then this can be rebased, and further fixes be made.

[cdwensley]

@fingolfin Why "use AutoDoc to extract examples"? That might be fine for new packages, but for one that has been around for a long time and has tests and manual files in parallel (but not identical) it seems unnecessary. Is it optional? If so, how to get rid of it?

[fingolfin]

@fingolfin Why "use AutoDoc to extract examples"? That might be fine for new packages, but for one that has been around for a long time and has tests and manual files in parallel (but not identical) it seems unnecessary.

I thought that PR #164 and #165 make it clear that it is necessary: humans make mistake, and are bad at keeping two different data sources in sync. They drift. As has happened here: this package had several outright broken examples in the manuals, that were not caught, because the tests did not carry the broken examples or carried variants without the breakage.

Is it optional? If so, how to get rid of it?

Of course it is optional: this is why xmod is not using it right now. But I think that's a mistake, for the reason explained above; and it is also very visible if you look into the failure logs.

That said: I don't mind if you prefer to close this PR, as long as you merge the other PRs, so that at least those issues in the xmod documentation are fixed. If you prefer to keep the remaining issues unfixed, that's fine by me :-)

[cdwensley]

You have convinced me. 164,165,166 are now merged. Now trying to implement 167 locally, so I have added to makedoc.g the extract_examples := rec( subdir := "tst/manual" ) but, when I run makedoc.g, the tests xmodn.tst are not created. Is there something else to be added?

[fingolfin]

@cdwensley Hmm, hard to say without seeing your file. Did you add it in the right place? Which GAP version are you using?

If you have and use the gh command line tool (which I recommend for many reasons), the easiest might be to simply check out this branch locally, by running inside a clone of this repository gh pr checkout 167.

In the meantime, here are the first few errors, copied and paste from the CI logs here, with some comments by me interspersed:

  ########> Diff in /tmp/gaproot/pkg/xmod/tst/manual/xmod02.tst:457
  # Input is:
  L18 := Cat1Select( 18 ); 
  # Expected output:
  #I  Usage:  Cat1Select( size, gpnum, num );  where gpnum <= 5
  fail
  # But found:
  fail
  ########
  ########> Diff in /tmp/gaproot/pkg/xmod/tst/manual/xmod02.tst:461
  # Input is:
  Cat1Select( 18, 4 );
  # Expected output:
  #I  Usage:  Cat1Select( size, gpnum, num );  where num <= 4
  fail
  # But found:
  fail
  ########

These two fail because the info level is not high enough (it probably is 0 but should be 1). These actually worked before I modified a test in one of the PRs by me you just merged: that test increased the info level to 1 but then "forgot" to decrease it again -- which is bad for this test, but it was necessary to fix a bunch of other tests.

Two solutions come to mind (I am sure there are more)

1. temporarily increase the info level for this test again
2. change the usage message to be printed as an error instead (that's what I'd prefer)

  ########> Diff in /tmp/gaproot/pkg/xmod/tst/manual/xmod02.tst:509
  # Input is:
  StructureDescription( C8 );
  # Expected output:
  [ "(S3 x D24) : C2", "D12" ]
  # But found:
  [ "(C3 x C3) : ((C2 x C2 x C2) : (C2 x C2))", "D12" ]
  ########

The output of StructureDescription is not stable and can change depending on the state of the random number generator, which packages are loaded, and changes in the GAP kernel. That said, it is fine for "easy" groups like dihedral and cyclic ones (which is why I didn't hesitate in my previous patches to make use of it); but this semi-direct product there is simply one which doesn't have a clear canonical description.

Some possible solutions:

1. don't use StructureDescription here (so: drop that call; or something simpler like Size; or ...)
2. use an easier example where StructureDescription is stable
3. don't include that example into the tests (turn it from <Example> into <Listing>) -- but of course that means it won't be checked anymore, so personally I'd try to avoid it (but sometimes it is the best compromise, given that we all have more important things to do ;-) )

  ########> Diff in /tmp/gaproot/pkg/xmod/tst/manual/xmod02.tst:560
  # Input is:
  GeneratorsOfGroup( obgp )[1];
  # Expected output:
  (11,13)>-(1,3)(5,6)(8,9)->(11,13)
  # But found:
  (11,13)>-(1,3)(4,6)(7,8)->(11,13)
  ########
  ########> Diff in /tmp/gaproot/pkg/xmod/tst/manual/xmod02.tst:562
  # Input is:
  Homset( gpd33, (11,12), (11,13) );
  # Expected output:
  <homset (11,12) -> (11,13) with head group Group(
  [ (11,12)>-(1,2)(5,6)(8,9)->(11,12), (11,12)>-(1,2)(4,5)(7,9)->(11,12) ] )>
  # But found:
  <homset (11,12) -> (11,13) with head group Group( 
  [ (11,12)>-(1,2)(4,6)(7,8)->(11,12) ] )>
  ########

This looks like it might be cause by some algorithms (in GAP?) now producing slightly different output.

  ########> Diff in /tmp/gaproot/pkg/xmod/tst/manual/xmod03.tst:17
  # Input is:
  mor5 := XModMorphism( X5, X5, sigma5, rho5 );
  # Expected output:
  [[c5->Aut(c5))] => [c5->Aut(c5))]] 
  # But found:
  Error, no method found! For debugging hints type ?Recovery from NoMe\
  thodFound
  Error, no 1st choice method found for `IsPreXModMorphism' on 1 arguments
  The 1st argument is 'fail' which might point to an earlier problem
  
  ########

No idea what is going on here, that sounds like a genuine problem only you can figure out.

Then after this, there are a ton of errors that are due to mor5 not being ignored -- this shows that it is usually best to fix these errors "from the top" so you don't waste time on something that's a consequence of an earlier failure.

  ########> Diff in /tmp/gaproot/pkg/xmod/tst/manual/xmod03.tst:19
  # Input is:
  Display( mor5 );
  # Expected output:
  Morphism of crossed modules :- 
  : Source = [c5->Aut(c5)] with generating sets:
    [ (5,6,7,8,9) ]
    [ GroupHomomorphismByImages( c5, c5, [ (5,6,7,8,9) ], [ (5,7,9,6,8) ] ) ]
  : Range = Source
  : Source Homomorphism maps source generators to:
    [ (5,9,8,7,6) ]
  : Range Homomorphism maps range generators to:
    [ GroupHomomorphismByImages( c5, c5, [ (5,6,7,8,9) ], [ (5,7,9,6,8) ] ) ]
  # But found:
  Error, Variable: 'mor5' must have a value
  ########
  ########> Diff in /tmp/gaproot/pkg/xmod/tst/manual/xmod03.tst:29