Added a helper function and updated the existing methods to preserve edge weights when removing vertices, edges or copying digraphs (issue #683) #877
Conversation
…edge weights when removing vertices, edges or copying digraphs (issue digraphs#683)
|
This looks like it's coming along. Could you please:
It'd be great if you could do this by next meeting, so we can do a proper review of your two PRs. |
|
Looks like linting is improved! If you could now add tests, that should satisfy the codecov check. |
…igraphRemoveVertex and related immutable operations. Also added testing.
mtorpey
left a comment
mtorpey
left a comment
There was a problem hiding this comment.
Subject to what we've said above, it'd be nice to see documentation. But this is coming along well.
| gap> m2!.edgeweights; | ||
| [ [ 5 ], [ 15 ], [ ], [ ] ] |
There was a problem hiding this comment.
There's a design decision here!
Either:
- mutable digraphs cannot hold EdgeWeights (because they can't hold attributes); or
- mutable digraphs can store edge weights internally using
!.edgeweightsas a workaround.
The second approach has been taken in this PR, and we should review whether it's a good idea when we finally merge.
|
Documentation should be installed in doc/z-chap5.xml |
| end); | ||
|
|
||
| InstallMethod(RemoveDigraphEdgeWeight, "for an immutable digraph, pos int, pos int", | ||
| [IsImmutableDigraph, IsPosInt, IsPosInt], |
There was a problem hiding this comment.
| [IsImmutableDigraph, IsPosInt, IsPosInt], | |
| [IsImmutableDigraph and HasEdgeWeights, IsPosInt, IsPosInt], |
| local newD, w; | ||
| newD := DigraphMutableCopy(D); | ||
|
|
||
| if HasEdgeWeights(D) then |
There was a problem hiding this comment.
| if HasEdgeWeights(D) then |
| function(D, v, pos) | ||
| local newD, w; | ||
| newD := DigraphMutableCopy(D); | ||
|
|
There was a problem hiding this comment.
| weights := EdgeWeightsMutableCopy(D); |
| [IsImmutableDigraph, IsPosInt, IsPosInt], | ||
| function(D, v, pos) | ||
| local newD, w; | ||
| newD := DigraphMutableCopy(D); |
There was a problem hiding this comment.
| newD := DigraphMutableCopy(D); | |
| newD := DigraphImmutableCopy(D); |
|
|
||
| MakeImmutable(newD); | ||
| if HasEdgeWeights(D) then | ||
| SetEdgeWeights(newD, StructuralCopy(newD!.edgeweights)); |
There was a problem hiding this comment.
| SetEdgeWeights(newD, StructuralCopy(newD!.edgeweights)); | |
| SetEdgeWeights(newD, weights)); |
3 participants
Issue -
https://github.com/digraphs/Digraphs/issues/683Previously, deleting vertices or edges from an EdgeWeightedDigraph or taking a mutable copy caused the resulting digraph to lose its edge weights. This commit is just a layout for which I'll add tests and debug.
Changes include:
CopyEdgeWeightsForSubdigraphhelper function to reconstruct weights after vertex removals.DigraphRemoveVertexto copy edge weights.DigraphRemoveEdgeto remove corresponding weight entries instead of dropping all weights.DigraphMutableCopyto copy edge weights when present.digraph.gd.