Change constructors for abstract lattices according to #557

[StevellM]

No description provided.

[codecov]

Codecov Report

Merging #609 (e041109) into master (1c23476) will decrease coverage by 0.11%.
The diff coverage is 94.07%.

@@            Coverage Diff             @@
##           master     #609      +/-   ##
==========================================
- Coverage   70.19%   70.08%   -0.12%     
==========================================
  Files         313      313              
  Lines       92043    92044       +1     
==========================================
- Hits        64611    64506     -105     
- Misses      27432    27538     +106     

Impacted Files	Coverage Δ
src/QuadForm/Database.jl	13.45% <0.00%> (-0.05%)	⬇️
src/QuadForm/Lattices.jl	82.66% <84.00%> (+0.06%)	⬆️
src/QuadForm/Herm/Lattices.jl	95.95% <100.00%> (+3.04%)	⬆️
src/QuadForm/IO.jl	90.97% <100.00%> (ø)
src/QuadForm/MassQuad.jl	71.18% <100.00%> (ø)
src/QuadForm/Quad/Genus.jl	94.33% <100.00%> (ø)
src/QuadForm/Quad/GenusRep.jl	88.23% <100.00%> (ø)
src/QuadForm/Quad/Lattices.jl	98.89% <100.00%> (+0.66%)	⬆️
src/QuadForm/Quad/Spaces.jl	89.92% <100.00%> (+<0.01%)	⬆️
src/QuadForm/Quad/ZLattices.jl	96.01% <100.00%> (ø)
... and 31 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 1c23476...e041109. Read the comment docs.

[StevellM]

With these new constructors, some HermLat and QuadLat functions in the respective ~/Types.jl are not used anymore. Should we keep them in case it is needed one day ? My feeling is like they are not necessary now, and they seem not to be type stable, for what I have seen while creating the few tests I have implemented. What do yout think @thofma @simonbrandhorst ?

[thofma]

If there is a accepting V::AbsSpace or L::AbsLat, how should V and L be referred to? I understand why @StevellM chose "abstract". I named the types in this way because they are the abstract types, but not sure we should refer to the mathematical objects in this way. I think I used to call them just "space" and "lattice". For the right definition of "hermitian" these are all hermitian, but this is too confusing I think :) Any thoughts?

[simonbrandhorst]

I would call AbsSpace the ambient space and L a lattice in V.

[StevellM]

The constructor for ZLat, I had conflicts with the functions when adding the check test for the rank of the matrices, because there are no such tests for Z-lattices. So I had to separate the constructors, and make sure that the creation for ZLat will never call the other ones. So I separate the case of QuadSpace over QQ and the other cases. For now that is the bast way I have found to solve this problem...

[thofma]

Hm, I see, thanks for the explanation. I think it would be a bit more clearer to just give the signature

function lattice(V::QuadSpace{FlintRationalField, fmpq_mat}, B::MatElem; isbasis::Bool = true)

a check keyword argument (which is not used).

For consistency the constructors should all have the same behaviour. There is something else that probably needs to be adjusted in the future, but adding the check here should be fine for now.

[StevellM]

Good, thanks for the suggestion. I will try to make this change soon.

[thofma]

I have tested it locally, if you want I can just push it.

[StevellM]

If it is not a problem for you, yes it could be nice :)

[thofma]

Done :)