Avoid using InstallValue

[fingolfin]

For some background, see gap-system/gap#1637

This is just the tip of the iceberg, many more (most? all?) InstallValue
calls in this repository could be eliminated

[zickgraf]

I have a question (sorry if I missed this while skipping over the linked issue): It seems like this also means getting rid of DeclareGlobalVariable. I think we use this to

1. add documentation for global variables via AutoDoc,
2. declare a variable in a gd file so it can be referenced a gi file before the variable is actually populated with a value (in a gi file that is read later).

What is the "official" way for doing those two things when not using DeclareGlobalVariable? Actually, the second point seems to make the CI fail for this PR.

[fingolfin]

If you need to reference a variable before actually defining it it, then GAP 4.12 (due to be released in 1-2 weeks) adds DeclareGlobalName as a weak alternative to DeclareGlobalValue -- all the new function does is to turn off the warnings one gets for referencing an undefined global. It does not provide a placeholder object.

Hence if you really access a global variable before referencing it this doesn't work (e.g. you do something like mylist[1] := not_yet_defined_global;). But I would advice against keeping such code. Luckily, usually it is not too hard to refactor to avoid DeclareGlobal* altogether. E.g.

• use ValueGlobal to access the global -- that's not create for code that has to be super-fast, but for initialization code it's usually no problem.
• reorder how files are read -- I am trying that with this PR right now

That leaves the documentation issue. AutoDoc >= 2020.08.11 knows about DeclareGlobalName -- but of course that first needs GAP 4.12, plus even after that is out, you may not want to require it right away... So another alternative is to just use a little AutoDoc trick: you can actually document commented out declarations. So you can write this:

#! @Description
#! Some global variable, bla bla bla
#DeclareGlobalVariable("bla bla");

and it will show up in the manual just as if there was no # in front of DeclareGlobalVariable

[codecov]

Codecov Report

Merging #483 (c7a931e) into master (001492c) will decrease coverage by 0.01%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #483      +/-   ##
==========================================
- Coverage   67.27%   67.26%   -0.02%     
==========================================
  Files         409      390      -19     
  Lines       70646    70616      -30     
==========================================
- Hits        47530    47501      -29     
+ Misses      23116    23115       -1     

Flag	Coverage Δ
4ti2Interface	49.68% <100.00%> (-0.11%)	⬇️
ExamplesForHomalg	84.67% <100.00%> (-0.13%)	⬇️
Gauss	80.20% <ø> (ø)
GaussForHomalg	88.28% <ø> (ø)
GradedModules	74.40% <ø> (ø)
GradedRingForHomalg	71.01% <ø> (+0.03%)	⬆️
HomalgToCAS	71.02% <ø> (ø)
IO_ForHomalg	67.19% <ø> (ø)
LocalizeRingForHomalg	61.09% <ø> (ø)
MatricesForHomalg	66.36% <ø> (ø)
Modules	71.85% <ø> (ø)
RingsForHomalg	42.18% <100.00%> (-0.26%)	⬇️
SCO	79.64% <ø> (ø)
ToolsForHomalg	69.77% <ø> (ø)
homalg	72.37% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
4ti2Interface/gap/4ti2Interface.gd	100.00% <ø> (ø)
ExamplesForHomalg/gap/ExamplesForHomalg.gd	100.00% <ø> (ø)
RingsForHomalg/gap/RingsForHomalg.gd	100.00% <ø> (ø)
4ti2Interface/gap/4ti2Interface.gi	39.26% <100.00%> (ø)
ExamplesForHomalg/gap/ExamplesForHomalg.gi	26.92% <100.00%> (ø)
RingsForHomalg/gap/GAPHomalg.gd	100.00% <100.00%> (ø)
RingsForHomalg/gap/GAPHomalg.gi	69.38% <100.00%> (-0.91%)	⬇️
RingsForHomalg/gap/GAPHomalgBasic.gi	76.47% <100.00%> (ø)
RingsForHomalg/gap/GAPHomalgBestBasis.gi	12.90% <100.00%> (ø)
RingsForHomalg/gap/GAPHomalgTools.gi	72.91% <100.00%> (ø)
... and 32 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 001492c...c7a931e. Read the comment docs.

[zickgraf]

@fingolfin Thanks for the detailed reply! :-) I like this new system and will apply it to my packages once I require GAP 4.12 (which should be rather soon after the release).

[fingolfin]

Fixed it

[fingolfin]

To make the tests pass, I've moved DeclareRepresentation calls to the .gd files (where they belong to anyway, at least per se the usual GAP conventions), and declared SageMacros in a .gd file, too. I hope both are fine with you.

[fingolfin]

The failure due to reduced code coverage is a red herring: I've also tweaked the SageMacros initialization to use a triple quoted multi line strings; this code is now counted as 30 lines, whereas before it treated those 30 lines as a single line due to the use of line continuations.

[mohamed-barakat]

Thank you very much Max.

[mohamed-barakat]

Do you want to bump the versions of the altered packages in your PR or shall I do this after merging?

[fingolfin]

As you prefer, but I am not near a computer right now, so I can't do anything for now

[mohamed-barakat]

I will try to push a commit on your PR branch, otherwise, I would ask you to bump the versions :)

[mohamed-barakat]

I will bump all versions after adding TestFile := "maketest.g"