Consider this:
gap> G:=HeisenbergPcpGroup(2);
Pcp-group with orders [ 0, 0, 0, 0, 0 ]
gap> Pcp([One(G)]);
Pcp [ id ] with orders [ fail ]
Oops.
The problem is that Pcp really expects a group as first argument, but we gave it a list. This is silently (accidentally?!) accepted anyway, and passed on to Cgs, which in the variant taking a list of elements assumes these already form an igs (with no checks for that). Since we gave it something that is not an igs....
It would be nice if this kind of problem was checked for (at least optionally, when requested by the user). Perhaps rename the current Cgs and Ngs to CgsNC and NgsNC and let their successors perform some argument validation?
As to Pcp, perhaps it should require that its first argument is a (pcp) group and reject anything else?
Consider this:
Oops.
The problem is that
Pcpreally expects a group as first argument, but we gave it a list. This is silently (accidentally?!) accepted anyway, and passed on toCgs, which in the variant taking a list of elements assumes these already form an igs (with no checks for that). Since we gave it something that is not an igs....It would be nice if this kind of problem was checked for (at least optionally, when requested by the user). Perhaps rename the current
CgsandNgstoCgsNCandNgsNCand let their successors perform some argument validation?As to
Pcp, perhaps it should require that its first argument is a (pcp) group and reject anything else?