NewMatrix(...,ncols,list) changed to NewMatrix(...,list,ncols)

[cdwensley]

There are four declared versions of Matrix( ..., list, ncols ) but there is NewMatrix( filt, R, ncols, list ).
This discrepancy is undesirable and should be changed before it gets too established.
Perhaps the experts will think it is already too tricky to change?
As to packages which might be affected:

• the intention was to provide a similar PR for cvec, but that proved too tricky - there are just two test cases and a few occurrence in cmat.gi;
• semigroups seems to just use Matrix;
• are there any others?

[cdwensley]

FininG uses NewMatrix a lot - investigating.

[codecov]

Codecov Report

❌ Patch coverage is 39.13043% with 14 lines in your changes missing coverage. Please review.
✅ Project coverage is 75.94%. Comparing base (b04e9ba) to head (9cac120).
⚠️ Report is 1 commits behind head on master.

Files with missing lines	Patch %	Lines
lib/matobj.gi	42.85%	8 Missing ⚠️
lib/matobjnz.gi	0.00%	3 Missing ⚠️
lib/matobjplist.gi	40.00%	3 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #6360      +/-   ##
==========================================
- Coverage   78.63%   75.94%   -2.69%     
==========================================
  Files         684      644      -40     
  Lines      292689   277950   -14739     
  Branches     8686     7883     -803     
==========================================
- Hits       230155   211103   -19052     
- Misses      60726    65309    +4583     
+ Partials     1808     1538     -270     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[fingolfin]

I agree to an extent that it would be preferable to have matching argument orders here.

But we also have to acknowledge that the status quo goes back to GAP 4.5 (!) so 14+ years. Besides FinInG, also cvec would be affected -- and potentially user code we don't know about.

So this is a major breaking change and not lightly taken, without a strong reason. I am not convinced this is the case here: wouldn't it suffice if we tell users "use Matrix, ignore NewMatrix, that one is only for people implementing new MatrixObj stuff", and perhaps adding a warning to the documentation for NewMatrix ?

Perhaps one could also allow both argument order, since no other argument is an integer (i.e. there should be no ambiguity)...

[cdwensley]

Thanks @fingolfin - I rather expected you to say that.
I like the idea of allowing both orders, and will add that to #6339.