Closed gap-package-distribution-bot[bot] closed 1 year ago
master
Testing: master/2023-09-01-21:43:35-9cb67296 vs master/2023-09-01-21:13:20-3ab20e58
Generated by Workflow: https://github.com/gap-system/PackageDistro/actions/runs/6054007103
In total, 156 packages were tested, out of which 151 succeeded, 1 failed and 4 were skipped.
1 package(s) failed tests also on the previous version.
151 package(s) succeeded tests also on the previous version.
4 package(s) skipped tests also on the previous version.
@ThomasBreuer apparently the new AtlasRep creates a read-only variable ZZ
which breaks the CAP/homalg test suites...
@fingolfin
ZZ
is a (documented) operation from the StandardFF package.
AtlasRep version 2.1.7 introduces a new dependency to StandardFF as a suggested package.
OK. I guess we can ask @zickgraf if they can avoid using ZZ in their test suite as a free variable.
I have created https://github.com/homalg-project/CAP_project/pull/1455 for CAP_project. Ping @mohamed-barakat for homalg_project.
However, I think it is not very polite of packages (or of GAP itself) to just "occupy" those short names like Z
, ZZ
, E
, EE
etc. Of course this makes it very convenient to write code using those functions, but I am always annoyed when trying to do something like X := ...; Y := ...; Z := ...;
in the REPL, where X
and Z
are already occupied. My workaround usually was XX := ...; YY := ...; ZZ := ...;
, which now also breaks. I would much prefer if packages would use longer names. If one really uses those functions excessively, one could always create a local alias with a short name, without polluting the global namespace.
@zickgraf
#@local
statement into the testfile in order to declare variables used later on as local. For example, the following testfile would work, no matter whether the StandardFF package is loaded or not.
#@local Z, ZZ
gap> START_TEST("test.tst");;
gap> Z:= 1;
1
gap> ZZ:= 1;
1
gap> Z = ZZ;
true
gap> STOP_TEST("test.tst");;
There is a statement in the GAP Tutorial that
The name of every global variable in the GAP library starts with a capital letter. Thus if you choose only names starting with a small letter for your own variables you will not attempt to overwrite any predefined variable. (Note that most of the predefined variables are read-only, and trying to change their values will result in an error message.)
Thus the suggestion is to use x
, y
, z
, etc. instead of X
, Y
, Z
, etc. as user variables.
Yes, this statement is well-hidden; I had thought that there is another instance somewhere in the Reference Manual, but I did not find it now.
I agree with both of you: it is annoying that X
, E
, Z
, ..., and now ZZ
are "blocked"; but also there is a clear rule to avoid this ("don't use variables starting with an upper case letter"), and a simple workaround for test files, too (namely using #@local
-- though it has the drawback that these examples then are not trivial to reproduce interactively).
Anyway, thanks @zickgraf for preparing a patch, and it seems this was already released, great. To be on the safe side long term, you may wish to rename ZZZ
to zz
or so... but for now it is of course perfectly fine 😃)
but also there is a clear rule to avoid ("don't use variables starting with an upper case letter")
Grepping in gap/lib
for "G:=", "U:=" one finds not only occurrences as local variables in the code but also in the GAPDoc sections, but maybe this GAPDoc code is not tested. So I assume the rule is a strong recommendation.
I am aware that these issues are consequences of GAP missing the concept of namespaces and that it would be a considerable amount of work to introduce them, so I am not complaining.
I will try to replace " ZZ" with " zz" in the homalg packages and see what happens.
The CAP fixes by @zickgraf helped but of course we still have failures in various homalg packages, so waiting for new release of these:
Our CIs are failing although they are not tested with LoadAllPackages( )
. This probably means that standard GAP is now marking ZZ
read-only. Is there a way to undo this change in the near future?
Side note: QPA2 is now also failing due to a conflict in the declaration of AsVector
.
I don't understand, I thought you replaced all uses of ZZ
?
Yes, we did in the deposited packages. I assumed this would be enough :)
@mohamed-barakat
This probably means that standard GAP is now marking
ZZ
read-only. Is there a way to undo this change in the near future?
I do not understand this statement.
The operation ZZ
belongs to the StandardFF package.
It is bound and then readonly as soon as this package is loaded.
Making ZZ
in StandardFF not readonly would be wrong.
The clash with user variables in CAP/homalg packages occurs since AtlasRep suggests loading StandardFF, thus I guess that "standard GAP" means GAP with some packages, one of them being AtlasRep. If one really wants to avoid loading StandardFF, one could try to load AtlasRep explicitly with the OnlyNeeded option, but for that one has to make sure that AtlasRep has not been loaded before.
The clash with user variables in CAP/homalg packages occurs since AtlasRep suggests loading StandardFF, thus I guess that "standard GAP" means GAP with some packages, one of them being AtlasRep.
Yes.
I do not understand this statement. The operation
ZZ
belongs to the StandardFF package. It is bound and then readonly as soon as this package is loaded. MakingZZ
in StandardFF not readonly would be wrong.
I was not suggesting making ZZ
read/write. I was asking if it would be possible to rename ZZ
in StandardFF into something different :) If the answer is no we have to rename all occurrences of ZZ
into something else in our non-deposited packages as well.
In the meanwhile, I have looked into StandardFF
and now understand the logic behind using ZZ
.
Side note: QPA2 is now also failing due to a conflict in the declaration of
AsVector
.
See https://github.com/frankluebeck/StandardFF/pull/6 for the clash of "AsVector" with QPA2. For now I will simply delete StandardFF from our CI completely.
Unfortunately QPA2 is not a deposited package and hence clashes in it with deposited packages are not detected. So in this case I am afraid the onus is on QPA2 maintainers to work something out with the StandardFF maintainer (Frank Lübeck) and/or to change their package.
PackageInfo.g
] [README
] [website] [source archive]