gap icon indicating copy to clipboard operation
gap copied to clipboard

Add IsVecOrMatObj, add filters to several MatrixObj implementations

Open fingolfin opened this issue 2 years ago • 2 comments

  • Add IsVecOrMatObj category
  • Move some declaration from matobj2.gd to matobj1.gd
  • Set various filters for MatrixObj implementations

Since I moved code and edited it, you may wish to look at the individual commits to see what exactly I changed.

This is motivated by https://github.com/gap-system/gap/pull/4935/files#r940054140

fingolfin avatar Aug 08 '22 15:08 fingolfin

@hulpke @frankluebeck this PR started out small and motivated by a change in Frank's PR, but it lead me to discover that FieldOfMatrixList was broken, and then some more changes ensued... Please review (I recommend looking at each commit separately, their commit message contain extra thoughts).

Maybe @ThomasBreuer also is interested in taking a look

fingolfin avatar Aug 12 '22 10:08 fingolfin

  • for now IsVecOrMatObj is intended to be a temporary workaround, removing it shouldn't be hard.
  • matobj1.gd and matobj2.gd are split for technical reasons: the first file provides definition that many other files need, and so have to be provided early; the other uses/references things that are provided by other .gd files that are loaded much later. So the split is needed unless one wants to severely reorder the order the other .gd files are loaded
  • FieldOfMatrixList is documented in .gd file with a GAPDoc documentation comment, which however is indeed not included in the reference manual -- not sure if that's intentional or not. Anyway, the comment outlines quite specifically what the intent of the function is; and indeed we use it with the expectation that it returns a minimal field containing all elements in various places.

fingolfin avatar Aug 12 '22 16:08 fingolfin

@frankluebeck @wucas @ThomasBreuer does this look OK now to you? further questions? I'd like to merge this ASAP, then we can possibly update PR #4983 or move forward in some other way so that we can get 4.12.0 out...

fingolfin avatar Aug 15 '22 08:08 fingolfin