Rename MultRowVector to MultVector #2709
Conversation
fingolfin
added
topic: documentation
fingolfin
requested review from
mohamed-barakat,
frankluebeck,
ssiccha and
ThomasBreuer
Codecov Report
@@ Coverage Diff @@
## master #2709 +/- ##
==========================================
- Coverage 76.03% 76.02% -0.01%
==========================================
Files 480 480
Lines 240919 240940 +21
==========================================
- Hits 183185 183184 -1
- Misses 57734 57756 +22
|
doc/ref/obsolete.xml
Outdated
| @@ -185,6 +189,11 @@ Instead of <C>PositionFirstComponent(list,obj)</C>, you may use | |||
| <C>PositionProperty(list,x->x[1]=obj)</C> as a replacement, depending on | |||
| your specific use case. | |||
|
|
|||
| <Index Key="MultRowVector"><C>MultRowVector</C></Index> | |||
| The five argument version of the operation <C>MultRowVector</C> has been | |||
| deprecated in GAP 4.8 since it was unused and only available for coefficient | |||
There was a problem hiding this comment.
GAP 4.8 should be replaced by GAP 4.10?
src/listoper.c
Outdated
| @@ -1571,33 +1571,50 @@ Obj FuncADD_ROW_VECTOR_2_FAST ( Obj self, | |||
|
|
|||
| /**************************************************************************** | |||
| ** | |||
| *F FuncMULT_ROW_VECTOR_2( <self>, <list>, <mult> ) | |||
| *F FuncMULT_VECTOR_LEFT_RIGHT_2( <self>, <list>, <mult> ) | |||
There was a problem hiding this comment.
The last argument is missing.
fingolfin
force-pushed
the
mh/MatrixObj-MultVector
branch
from
8ea9103
to
1e466c6
Compare
|
@ssiccha Sergio, could you please indicate whether you think it's reasonable if I squash this into a single commit? |
fingolfin
force-pushed
the
mh/MatrixObj-MultVector
branch
from
1e466c6
to
e317c68
Compare
|
Squashed that one commit, rebased, and addressed @ThomasBreuer other remark. |
|
Will the identifier |
|
We never remove an identifier that has been made obsolete before at least all distributed packages stopped using it. Beyond that, we might remove it -- but if you have special need to keep it around, of course that'd be considered, too; and e.g. a comment noting this requirement could be added next to |
|
@ThomasBreuer is this PR OK now, from your perspective? If so, please approve (or at least drop the "request for changes"). Thanks! |
|
At this point, I think it might be best to not rush this into GAP 4.10 at the last minute, but rather delay it to GAP 4.11. Setting milestone accordingly. Feel free to override me if you think it's important to get this in now, though. |
- Split MultRowVector to MultRowVectorLeft / MultRowVectorRight.
- MultRowVector is declared a synonym for MultRowVectorLeft.
- MultRowVectorOp is renamed to MultRowVectorLeftOp on the C level.
- Add four argument declarations for MultRowVectorLeft( vec, mul, from, to )
and ..Right
- Remove the five argument version of MultRowVector in MatrixObj.
Deprecate it also in the library. This version was not used anywhere in
the library or packages
- Add generic and plist implementations of MultRowVector where necessary
- Implement and declare ..Left and ..Right versions in matrixobj*
- Only do the ..Left version in listcoef.g?. The methods from listcoef.gi
are only used for integers and FFEs in the library.
Adding the ..Right versions would only introduce lots of unused code.
People wanting to use the ..Right versions should use vector objs.
- Have a kernel function MULT_ROW_VECTOR_LEFT_RIGHT_2 which is called by
MULT_ROW_VECTOR_LEFT_2 and ..RIGHT_2
- Add tests for MultRowVector
Performs the following sed commands in the directories
`lib/`, `src/`, `hpcgap/`. Then undo the changes in
the `obsolete.{gd,gi}` files.
sed -i 's/MULT_ROW_VECTOR/MULT_VECTOR/g' `find . -type f`
sed -i 's/MULT_ROWVECTOR/MULT_VECTOR/g' `find . -type f`
sed -i 's/MultRowVector/MultVector/g' `find . -type f`
git checkout -- obsolete.{gd,gi}
Also perform:
sed -i 's/MultRowVector/MultVector/g' doc/ref/vector.xml
fingolfin
force-pushed
the
mh/MatrixObj-MultVector
branch
from
e317c68
to
6fa2c18
Compare
fingolfin
changed the title
fingolfin
added
release notes: added
With PR #2640 merged, I am submitting the remaining MatrixObj changes as separate PRs.
This contains @ssiccha work on
MultRowVectorresp.MultVectorfrom PR #1685. I think these change are fine, but perhaps we want to squash them into a single commit (and/or split them differently across commits than right now). Also, the changes are a bit more extensive, and I felt it was best to give people a chance to review them separately once again (back then, @frankluebeck briefly commented on the PR, but I think I was the only one to extensively review it.@ssiccha feel free to approve, even though these are your own changes ;-). In fact, that means you are uniquely qualified to review this now ;-).