Added error handling to DigraphRemoveEdge(s) and DigraphRemoveVertex(ices) operations. Also added tests for (#issue863)

doc/oper.xml

@@ -709,22 +709,22 @@ true
     the edge is returned.
     <P/>
 
-    Note that if <A>digraph</A> belongs to <Ref Filt="IsImmutableDigraph"/>,
-    then a new copy of <A>digraph</A> will be returned even if <A>edge</A> or 
-    <A>[src, ran]</A> does not define an edge of <A>digraph</A>.<P/>
+    An error is raised if <A>edge</A> or <A>[src, ran]</A> does not define an
+    edge of <A>digraph</A>.<P/>
 
     <Example><![CDATA[
 gap> D := CycleDigraph(250000);
 <immutable cycle digraph with 250000 vertices>
 gap> D := DigraphRemoveEdge(D, [250000, 1]);
 <immutable digraph with 250000 vertices, 249999 edges>
-gap> new := DigraphRemoveEdge(D, [25000, 2]);;
+gap> new := DigraphRemoveEdge(D, [1, 2]);
+<immutable digraph with 250000 vertices, 249998 edges>
 gap> new = D;
-true
+false
 gap> IsIdenticalObj(new, D);
 false
 gap> D := DigraphMutableCopy(D);;
-gap> new := DigraphRemoveEdge(D, 2500, 2);;
+gap> new := DigraphRemoveEdge(D, 2, 3);;
 gap> IsIdenticalObj(new, D);
 true]]>
 </Example>
@@ -756,10 +756,8 @@ true]]>
       Filt="IsImmutableDigraph"/>, the edge is removed from an immutable copy of
     <A>digraph</A> and this new digraph is returned.<P/>
 
-    Note that if <A>edges</A> is empty, then this operation
-    will always return <A>digraph</A> rather than a copy. Also, if any element
-    of <A>edges</A> is invalid (i.e. does not define an edge of <A>digraph</A>)
-    then that element will simply be ignored.
+    An error is raised if any element of <A>edges</A> is invalid (i.e does not 
+    define an edge of <A>digraph</A>).
 
     <Example><![CDATA[
 gap> D := CycleDigraph(250000);
@@ -768,8 +766,8 @@ gap> D := DigraphRemoveEdges(D, [[250000, 1]]);
 <immutable digraph with 250000 vertices, 249999 edges>
 gap> D := DigraphMutableCopy(D);
 <mutable digraph with 250000 vertices, 249999 edges>
-gap> new := DigraphRemoveEdges(D, [[1, 2], [2, 3], [3, 100]]);
-<mutable digraph with 250000 vertices, 249997 edges>
+gap> new := DigraphRemoveEdges(D, [[1, 2], [2, 3], [3, 4]]);
+<mutable digraph with 250000 vertices, 249996 edges>
 gap> new = D;
 true
 ]]></Example>

gap/attr.gi

@@ -1967,8 +1967,10 @@ function(D)
           if path <> fail then
             for i in [1 .. (Length(path[1]) - 1)] do
               # remove edges corresponding to the current path
-              digraphCopy := DigraphRemoveEdge(digraphCopy,
-                [path[1][i], path[1][i + 1]]);
+              if [path[1][i], path[1][i + 1]] in DigraphEdges(digraphCopy) then
+                digraphCopy := DigraphRemoveEdge(digraphCopy,
+                  [path[1][i], path[1][i + 1]]);
+              fi;
               # add backward edges, if they are missing
               if not [path[1][i + 1], path[1][i]] in
                     DigraphEdges(digraphCopy) then
@@ -2027,8 +2029,12 @@ function(D)
     for cutEdges in permutations do
       digraphCopy := DigraphMutableCopy(pathInducedSubgraph);
       for edge in cutEdges do
-        digraphCopy := DigraphRemoveEdge(digraphCopy, edge[1], edge[2]);
-        digraphCopy := DigraphRemoveEdge(digraphCopy, edge[2], edge[1]);
+        if [edge[1], edge[2]] in DigraphEdges(digraphCopy) then
+          digraphCopy := DigraphRemoveEdge(digraphCopy, edge[1], edge[2]);
+        fi;
+        if [edge[2], edge[1]] in DigraphEdges(digraphCopy) then
+          digraphCopy := DigraphRemoveEdge(digraphCopy, edge[2], edge[1]);
+        fi;
       od;
       if not IsConnectedDigraph(digraphCopy) then
         component1 := InducedSubdigraph(D,

gap/oper.gi

@@ -109,7 +109,7 @@ InstallMethod(DigraphRemoveVertex,
 function(D, u)
   local pos, w, v;
   if u > DigraphNrVertices(D) then
-    return D;
+    ErrorNoReturn("the vertex ", u, " does not exist,");
   fi;
   RemoveDigraphVertexLabel(D, u);
   if IsBound(D!.edgelabels) then
@@ -139,7 +139,7 @@ InstallMethod(DigraphRemoveVertex,
 [IsImmutableDigraph, IsPosInt],
 function(D, u)
   if u > DigraphNrVertices(D) then
-    return D;
+    ErrorNoReturn("the vertex ", u, " does not exist,");
   fi;
   return MakeImmutable(DigraphRemoveVertex(DigraphMutableCopy(D), u));
 end);
@@ -151,7 +151,13 @@ function(D, list)
   if not IsDuplicateFreeList(list) or not ForAll(list, IsPosInt) then
     ErrorNoReturn("the 2nd argument <list> must be a ",
                   "duplicate-free list of positive integers,");
-  elif not IsMutable(list) then
+  fi;
+  for v in list do
+    if v > DigraphNrVertices(D) then
+      ErrorNoReturn("the vertex ", v, " does not exist,");
+    fi;
+  od;
+  if not IsMutable(list) then
     list := ShallowCopy(list);
   fi;
   # The next line is essential since otherwise removing the 1st node,
@@ -239,10 +245,11 @@ function(D, src, ran)
                   "digraph <D> that is the 1st argument,");
   fi;
   pos := Position(D!.OutNeighbours[src], ran);
-  if pos <> fail then
-    Remove(D!.OutNeighbours[src], pos);
-    RemoveDigraphEdgeLabel(D, src, pos);
+  if pos = fail then
+    ErrorNoReturn("the edge [", src, ", ", ran, "] does not exist,");
   fi;
+  Remove(D!.OutNeighbours[src], pos);
+  RemoveDigraphEdgeLabel(D, src, pos);
   return D;
 end);
 

tst/standard/attr.tst

@@ -3140,7 +3140,7 @@ gap> D := DigraphAddEdge(D, 1, 3);
 gap> D := DigraphRemoveEdge(D, 1, 3);
 <immutable digraph with 6 vertices, 11 edges>
 gap> D := DigraphRemoveEdge(D, 1, 3);
-<immutable digraph with 6 vertices, 11 edges>
+Error, the edge [1, 3] does not exist,
 
 # DigraphVertexConnectivity
 gap> D := Digraph([[2, 3, 4], [3, 4], [4], []]);

tst/standard/oper.tst

@@ -63,9 +63,7 @@ false
 gap> gr := Digraph([[2], []]);
 <immutable digraph with 2 vertices, 1 edge>
 gap> DigraphRemoveEdges(gr, [[2, 1]]);
-<immutable digraph with 2 vertices, 1 edge>
-gap> last = gr;
-true
+Error, the edge [2, 1] does not exist,
 gap> DigraphRemoveEdges(gr, [[1, 2]]);
 <immutable empty digraph with 2 vertices>
 gap> gr := DigraphFromDigraph6String("&DtGsw_");
@@ -110,6 +108,88 @@ gap> gr := DigraphRemoveEdge(gr, [2, 1]);
 gap> DigraphEdges(gr);
 [ [ 1, 2 ] ]
 
+# Testing DigraphRemoveEdge(s) and DigraphRemoveVertex(ices) correctly handles valid removals and raises errors for invalid ones.
+gap> gr := Digraph([[2], [1]]);
+<immutable digraph with 2 vertices, 2 edges>
+gap> gr2 := DigraphRemoveEdge(gr, [1, 2]);
+<immutable digraph with 2 vertices, 1 edge>
+gap> DigraphEdges(gr2);
+[ [ 2, 1 ] ]
+gap> D := DigraphMutableCopy(Digraph([[2, 3], [1], [1]]));
+<mutable digraph with 3 vertices, 4 edges>
+gap> DigraphRemoveEdge(D, [1, 3]);
+<mutable digraph with 3 vertices, 3 edges>
+gap> DigraphEdges(D);
+[ [ 1, 2 ], [ 2, 1 ], [ 3, 1 ] ]
+gap> gr := Digraph([[2], [1]]);
+<immutable digraph with 2 vertices, 2 edges>
+gap> DigraphRemoveEdge(gr, [1, 1]);
+Error, the edge [1, 1] does not exist,
+gap> D := DigraphMutableCopy(gr);
+<mutable digraph with 2 vertices, 2 edges>
+gap> DigraphRemoveEdge(D, [2, 2]);
+Error, the edge [2, 2] does not exist,
+gap> gr := Digraph([[2, 3], [1], [1]]);
+<immutable digraph with 3 vertices, 4 edges>
+gap> gr2 := DigraphRemoveEdges(gr, [[1, 2], [3, 1]]);
+<immutable digraph with 3 vertices, 2 edges>
+gap> DigraphEdges(gr2);
+[ [ 1, 3 ], [ 2, 1 ] ]
+gap> D := DigraphMutableCopy(Digraph([[2, 3], [1, 3], []]));
+<mutable digraph with 3 vertices, 4 edges>
+gap> DigraphRemoveEdges(D, [[1, 2], [2, 3]]);
+<mutable digraph with 3 vertices, 2 edges>
+gap> DigraphEdges(D);
+[ [ 1, 3 ], [ 2, 1 ] ]
+gap> gr := Digraph([[2, 3], [1], [1]]);
+<immutable digraph with 3 vertices, 4 edges>
+gap> DigraphRemoveEdges(gr, [[1, 1]]);
+Error, the edge [1, 1] does not exist,
+gap> D := DigraphMutableCopy(gr);
+<mutable digraph with 3 vertices, 4 edges>
+gap> DigraphRemoveEdges(D, [[2, 3]]);
+Error, the edge [2, 3] does not exist,
+gap> gr := Digraph([[2, 3], [1, 3], [1]]);
+<immutable digraph with 3 vertices, 5 edges>
+gap> gr2 := DigraphRemoveVertex(gr, 2);
+<immutable digraph with 2 vertices, 2 edges>
+gap> DigraphEdges(gr2);
+[ [ 1, 2 ], [ 2, 1 ] ]
+gap> D := DigraphMutableCopy(Digraph([[2], [1]]));
+<mutable digraph with 2 vertices, 2 edges>
+gap> DigraphRemoveVertex(D, 1);
+<mutable empty digraph with 1 vertex>
+gap> DigraphEdges(D);
+[  ]
+gap> gr := Digraph([[2], [1]]);
+<immutable digraph with 2 vertices, 2 edges>
+gap> DigraphRemoveVertex(gr, 5);
+Error, the vertex 5 does not exist,
+gap> D := DigraphMutableCopy(gr);
+<mutable digraph with 2 vertices, 2 edges>
+gap> DigraphRemoveVertex(D, 3);
+Error, the vertex 3 does not exist,
+gap> gr := CompleteDigraph(4);
+<immutable complete digraph with 4 vertices>
+gap> gr2 := DigraphRemoveVertices(gr, [1, 3]);
+<immutable digraph with 2 vertices, 2 edges>
+gap> DigraphEdges(gr2);
+[ [ 1, 2 ], [ 2, 1 ] ]
+gap> D := DigraphMutableCopy(CompleteDigraph(3));
+<mutable digraph with 3 vertices, 6 edges>
+gap> DigraphRemoveVertices(D, [1, 2]);
+<mutable empty digraph with 1 vertex>
+gap> DigraphEdges(D);
+[  ]
+gap> gr := CompleteDigraph(4);
+<immutable complete digraph with 4 vertices>
+gap> DigraphRemoveVertices(gr, [1, 5]);
+Error, the vertex 5 does not exist,
+gap> D := DigraphMutableCopy(gr);
+<mutable digraph with 4 vertices, 12 edges>
+gap> DigraphRemoveVertices(D, [2, 10]);
+Error, the vertex 10 does not exist,
+
 # Tests for digraph operator "^" (implements D ^ p and D ^ t using OnDigraphs)
 gap> D := CycleDigraph(5);
 <immutable cycle digraph with 5 vertices>
@@ -795,7 +875,7 @@ gap> DigraphRemoveVertex(gr, 0);
 Error, no method found! For debugging hints type ?Recovery from NoMethodFound
 Error, no 1st choice method found for `DigraphRemoveVertex' on 2 arguments
 gap> DigraphRemoveVertex(gr, 15);
-<immutable digraph with 14 vertices, 54 edges>
+Error, the vertex 15 does not exist,
 gap> gr2 := DigraphRemoveVertex(gr, 10);;
 gap> DigraphNrVertices(gr2);
 13
@@ -829,7 +909,7 @@ gap> gr2 := DigraphRemoveVertices(gr, [1, 0]);
 Error, the 2nd argument <list> must be a duplicate-free list of positive integ\
 ers,
 gap> gr2 := DigraphRemoveVertices(gr, [1, 5]);
-<immutable digraph with 3 vertices, 6 edges>
+Error, the vertex 5 does not exist,
 gap> gr2 := DigraphRemoveVertices(gr, [1, 3]);
 <immutable digraph with 2 vertices, 2 edges>
 gap> IsCompleteDigraph(gr2);
@@ -2907,8 +2987,8 @@ Error, an element of the 2nd argument (roots) is not a vertex of the 1st argum\
 ent (a digraph)
 gap> D3 := CompleteDigraph(7);
 <immutable complete digraph with 7 vertices>
-gap> D3_edges := [1 .. 7];
-[ 1 .. 7 ]
+gap> D3_edges := [2 .. 7];
+[ 2 .. 7 ]
 gap> for i in D3_edges do
 >      D3 := DigraphRemoveEdge(D3, [1, i]);
 >      D3 := DigraphRemoveEdge(D3, [i, 1]);

tst/testinstall.tst

@@ -247,7 +247,7 @@ gap> gr2 := DigraphClosure(gr, 7);;
 gap> gr = gr2;
 true
 gap> gr := DigraphRemoveEdge(gr, [1, 2]);;
-gap> gr := DigraphRemoveEdges(gr, [[1, 2], [2, 1]]);;
+gap> gr := DigraphRemoveEdges(gr, [[2, 1]]);;
 gap> DigraphNrEdges(gr);
 40
 gap> DigraphNrAdjacencies(gr);