Include support for removal of vertex sequences

This fixes #189

doc/quickstart.rst

@@ -143,6 +143,22 @@ descriptors:
    >>> print(len(list(vlist)))
    10
 
+Each vertex in a graph has an unique index, which is *always* between
+:math:``0`` and :math:``N-1``, where :math:``N`` is the number of
+vertices. This index can be obtained by using the
+:attr:`~graph_tool.Graph.vertex_index` attribute of the graph (which is
+a *property map*, see :ref:`sec_property_maps`), or by converting the
+vertex descriptor to an ``int``.
+
+.. doctest::
+
+   >>> v = g.add_vertex()
+   >>> print(g.vertex_index[v])
+   11
+   >>> print(int(v))
+   11
+
+   
 Edges and vertices can also be removed at any time with the
 :meth:`~graph_tool.Graph.remove_vertex` and :meth:`~graph_tool.Graph.remove_edge` methods,
 
@@ -151,21 +167,48 @@ Edges and vertices can also be removed at any time with the
    >>> g.remove_edge(e)                               # e no longer exists
    >>> g.remove_vertex(v2)                # the second vertex is also gone
 
+   
 .. note::
 
-   Removing a vertex is an :math:`O(N)` operation. The vertices are
-   internally stored in a `STL vector <http://en.wikipedia.org/wiki/Vector_%28STL%29>`_,
-   so removing an element somewhere in the middle of the list requires
-   the shifting of the rest of the list. Thus, fast :math:`O(1)`
-   removals are only possible either if one can guarantee that only
-   vertices in the end of the list are removed (the ones last added to
-   the graph), or if the relative vertex ordering is invalidated. This
-   last behavior can be achieved by passing the option ``fast == True``,
-   to :meth:`~graph_tool.Graph.remove_vertex`, which causes vertex
+   Removing a vertex is typically an :math:`O(N)` operation. The
+   vertices are internally stored in a `STL vector
+   <http://en.wikipedia.org/wiki/Vector_%28STL%29>`_, so removing an
+   element somewhere in the middle of the list requires the shifting of
+   the rest of the list. Thus, fast :math:`O(1)` removals are only
+   possible either if one can guarantee that only vertices in the end of
+   the list are removed (the ones last added to the graph), or if the
+   relative vertex ordering is invalidated. This last behavior can be
+   achieved by passing the option ``fast == True``, to
+   :meth:`~graph_tool.Graph.remove_vertex`, which causes the vertex
    being deleted to be 'swapped' with the last vertex (i.e. with the
    largest index), which will in turn inherit the index of the vertex
    being deleted.
 
+.. warning::
+
+   Because of the above, removing a vertex with an index smaller than
+   :math:`N-1` will **invalidate either the last** (``fast = True``)
+   **or all** (``fast = False``) **descriptors pointing to vertices with
+   higher index**.
+
+   As a consequence, if more than one vertex is to be removed at a given
+   time, they should **always** be removed in decreasing index order:
+   
+   .. code::
+
+       # 'del_list' is a list of vertex descriptors
+       for v in reversed(sorted(del_list)):
+           g.remove_vertex(v)
+
+   Alternatively (and preferably), a list (or any iterable) may be
+   passed directly as the ``vertex`` parameter of the
+   :meth:`~graph_tool.Graph.remove_vertex` function, and the above is
+   performed internally (in C++).
+
+   Note that property map values (see :ref:`sec_property_maps`) are
+   unaffected by the index changes due to vertex removal.
+
+.. note::
 
    Removing an edge is an :math:`O(k_{s} + k_{t})` operation, where
    :math:`k_{s}` is the out-degree of the source vertex, and
@@ -174,26 +217,7 @@ Edges and vertices can also be removed at any time with the
    `True`, in which case it becomes :math:`O(1)`, at the expense of
    additional data of size :math:`O(E)`.
 
-Each vertex in a graph has an unique index, which is numbered from 0 to
-N-1, where N is the number of vertices. This index can be obtained by
-using the :attr:`~graph_tool.Graph.vertex_index` attribute of the graph
-(which is a *property map*, see :ref:`sec_property_maps`), or by
-converting the vertex descriptor to an ``int``.
-
-.. doctest::
-
-   >>> v = g.add_vertex()
-   >>> print(g.vertex_index[v])
-   11
-   >>> print(int(v))
-   11
-
-.. note::
-
-   Removing a vertex will cause the index of any vertex with a larger
-   index to be changed, so that the indexes *always* conform to the
-   :math:`[0, N-1]` range. However, property map values (see
-   :ref:`sec_property_maps`) are unaffected.
+   No edge descriptors are ever invalidated after edge removal.
 
 Since vertices are uniquely identifiable by their indexes, there is no
 need to keep the vertex descriptor lying around to access them at a
@@ -238,6 +262,7 @@ its index will be "vacant", and the remaining indexes will be left
 unmodified, and thus will not lie in the range :math:`[0, E-1]`.  If a
 new edge is added, it will reuse old indexes, in an increasing order.
 
+
 .. _sec_iteration:
 
 Iterating over vertices and edges
@@ -306,7 +331,7 @@ vertices in the graph.
    somewhere (such as in a list) and remove them only after no iterator
    is being used. Removal during iteration will cause bad things to
    happen.
-
+   
 
 .. _sec_property_maps:
 

src/graph/graph.hh

@@ -101,8 +101,8 @@ public:
     void PurgeEdges();    // removes filtered edges
     void Clear();
     void ClearEdges();
-    void ShiftVertexProperty(boost::any map, size_t index) const;
-    void MoveVertexProperty(boost::any map, size_t index) const;
+    void ShiftVertexProperty(boost::any map, boost::python::object oindex) const;
+    void MoveVertexProperty(boost::any map, boost::python::object oindex) const;
     void ReIndexVertexProperty(boost::any map, boost::any old_index) const;
     void CopyVertexProperty(const GraphInterface& src, boost::any prop_src,
                             boost::any prop_tgt);

src/graph/graph_properties.cc

@@ -45,15 +45,20 @@ const char* type_names[] =
 
 struct shift_vertex_property
 {
-    template <class PropertyMap>
+    template <class PropertyMap, class Vec>
     void operator()(PropertyMap, const GraphInterface::multigraph_t& g,
-                    boost::any map, size_t vi, bool& found) const
+                    boost::any map, const Vec& vi, bool& found) const
     {
         try
         {
             PropertyMap pmap = any_cast<PropertyMap>(map);
-            for (size_t i = vi; i < num_vertices(g)-1; ++i)
-                pmap[vertex(i,g)] = pmap[vertex(i+1,g)];
+            size_t back = num_vertices(g) - 1;
+            for (auto v : vi)
+            {
+                for (size_t i = v; i < back; ++i)
+                    pmap[vertex(i, g)] = pmap[vertex(i + 1, g)];
+                back--;
+            }
             found = true;
         }
         catch (bad_any_cast&) {}
@@ -61,8 +66,9 @@ struct shift_vertex_property
 };
 
 // this function will shift all the properties when a vertex is to be deleted
-void GraphInterface::ShiftVertexProperty(boost::any prop, size_t index) const
+void GraphInterface::ShiftVertexProperty(boost::any prop, python::object oindex) const
 {
+    boost::multi_array_ref<int64_t,1> index = get_array<int64_t,1>(oindex);
     bool found = false;
     mpl::for_each<writable_vertex_properties>
         (std::bind(shift_vertex_property(), std::placeholders::_1, std::ref(*_mg),
@@ -73,14 +79,19 @@ void GraphInterface::ShiftVertexProperty(boost::any prop, size_t index) const
 
 struct move_vertex_property
 {
-    template <class PropertyMap>
+    template <class PropertyMap, class Vec>
     void operator()(PropertyMap, const GraphInterface::multigraph_t& g,
-                    boost::any map, size_t vi, size_t back, bool& found) const
+                    boost::any map, const Vec& vi, size_t back,
+                    bool& found) const
     {
         try
         {
             PropertyMap pmap = any_cast<PropertyMap>(map);
-            pmap[vertex(vi,g)] = pmap[vertex(back,g)];
+            for (auto v : vi)
+            {
+                pmap[vertex(v, g)] = pmap[vertex(back, g)];
+                back--;
+            }
             found = true;
         }
         catch (bad_any_cast&) {}
@@ -88,8 +99,9 @@ struct move_vertex_property
 };
 
 // this function will move the back of the property map when a vertex in the middle is to be deleted
-void GraphInterface::MoveVertexProperty(boost::any prop, size_t index) const
+void GraphInterface::MoveVertexProperty(boost::any prop, python::object oindex) const
 {
+    boost::multi_array_ref<int64_t,1> index = get_array<int64_t,1>(oindex);
     size_t back = num_vertices(*_mg) - 1;
     bool found = false;
     mpl::for_each<writable_vertex_properties>

src/graph/graph_python_interface.cc

@@ -142,33 +142,21 @@ python::object add_vertex(python::object g, size_t n)
     return python::object(PythonVertex(g, add_vertex(gi.GetGraph())));
 }
 
-struct shift_vertex_property
-{
-    template <class Graph, class PropertyMap>
-    void operator()(const Graph& g, size_t vi, PropertyMap pmap)
-        const
-    {
-        size_t N = num_vertices(g);
-        if (N <= 1 || vi >= N - 1)
-            return;
-        for (size_t i = vi; i < N-1; ++i)
-        {
-            pmap[vertex(i, g)] = pmap[vertex(i+1, g)];
-        }
-    }
-};
 
-void remove_vertex(GraphInterface& gi, const python::object& v, bool fast)
+void remove_vertex(GraphInterface& gi, const python::object& oindex, bool fast)
 {
-    PythonVertex& pv = python::extract<PythonVertex&>(v);
-    pv.CheckValid();
-    GraphInterface::vertex_t dv = pv.GetDescriptor();
-    pv.SetValid(false);
-
+    boost::multi_array_ref<int64_t,1> index = get_array<int64_t,1>(oindex);
+    auto& g = gi.GetGraph();
     if (fast)
-        remove_vertex_fast(dv, gi.GetGraph());
+    {
+        for (auto v : index)
+            remove_vertex_fast(vertex(v, g), g);
+    }
     else
-        remove_vertex(dv, gi.GetGraph());
+    {
+        for (auto v : index)
+            remove_vertex(vertex(v, g), g);
+    }
 }
 
 struct add_new_edge

src/graph_tool/__init__.py

@@ -1039,7 +1039,7 @@ def edge_endpoint_property(g, prop, endpoint, eprop=None):
 @_limit_args({"htype": ["int8_t", "int32_t", "int64_t"]})
 def perfect_prop_hash(props, htype="int32_t"):
     """Given a list of property maps `props` of the same type, a derived list of
-    property maps with integral type `htype` is retured, where each value is
+    property maps with integral type `htype` is returned, where each value is
     replaced by a perfect (i.e. unique) hash value.
 
     .. note::
@@ -1431,7 +1431,8 @@ class Graph(object):
             return (self.vertex(i) for i in range(pos, pos + n))
 
     def remove_vertex(self, vertex, fast=False):
-        r"""Remove a vertex from the graph.
+        r"""Remove a vertex from the graph. If ``vertex`` is an iterable, it
+        should correspond to a sequence of vertices to be removed.
 
         .. note::
 
@@ -1441,6 +1442,27 @@ class Graph(object):
            degree of the vertex being deleted, and :math:`k_{\text{last}}` is
            the (total) degree of the vertex with the largest index.
 
+        .. warning::
+
+           This operation may invalidate vertex descriptors. Vertices are always
+           indexed contiguously in the range :math:`[0, N-1]`, hence vertex
+           descriptors with an index higher than ``vertex`` will be invalidated
+           after removal (if ``fast == False``, otherwise only descriptors
+           pointing to vertices with the largest index will be invalidated).
+
+           Because of this, the only safe way to remove more than one vertex at
+           once is to sort them in decreasing index order:
+
+           .. code::
+
+               # 'del_list' is a list of vertex descriptors
+               for v in reversed(sorted(del_list)):
+                   g.remove_vertex(v)
+
+           Alternatively (and preferably), a list (or iterable) may be passed
+           directly as the ``vertex`` parameter, and the above is performed
+           internally (in C++).
+
         .. warning::
 
            If ``fast == True``, the vertex being deleted is 'swapped' with the
@@ -1451,20 +1473,35 @@ class Graph(object):
 
         """
         self.__check_perms("del_vertex")
-        vertex = self.vertex(int(vertex))
-        index = self.vertex_index[vertex]
+
+        try:
+            vs = numpy.array([int(vertex)], dtype="int64")
+        except TypeError:
+            try:
+                vs = numpy.asarray(vertex, dtype="int64")
+            except TypeError:
+                vs = numpy.asarray([int(v) for v in vertex], dtype="int64")
+
+            if len(vs) == 0:
+                return
+
+            vs = numpy.sort(vs)[::-1]
+
         back = self.__graph.GetNumberOfVertices(False) - 1
 
+        if vs.max() > back:
+            raise ValueError("Vertex index %d is invalid" % vs.max())
+
         # move / shift all known property maps
-        if index != back:
+        if len(vs) > 0 or vs[0] != back:
             for pmap in self.__known_properties.values():
                 if pmap() is not None and pmap().key_type() == "v" and pmap().is_writable():
                     if fast:
-                        self.__graph.MoveVertexProperty(pmap()._PropertyMap__map.get_map(), index)
+                        self.__graph.MoveVertexProperty(pmap()._PropertyMap__map.get_map(), vs)
                     else:
-                        self.__graph.ShiftVertexProperty(pmap()._PropertyMap__map.get_map(), index)
+                        self.__graph.ShiftVertexProperty(pmap()._PropertyMap__map.get_map(), vs)
 
-        libcore.remove_vertex(self.__graph, vertex, fast)
+        libcore.remove_vertex(self.__graph, vs, fast)
 
     def clear_vertex(self, vertex):
         """Remove all in and out-edges from the given vertex."""