docs: cleanups, clarify (+test) connected components (#180)

Most of this relates to the behaviour on connected components (the
correct name - each component is connected), but a few other small doc
fixes as, erm, I've not looked at any of the code for awhile....

src/view.rs

@@ -33,7 +33,7 @@ pub trait PortView {
     #[must_use]
     fn port_offset(&self, port: impl Into<PortIndex>) -> Option<PortOffset>;
 
-    /// Returns the port index for a given node, direction, and offset.
+    /// Returns the port index for a given node and offset.
     #[must_use]
     fn port_index(&self, node: NodeIndex, offset: PortOffset) -> Option<PortIndex>;
 
@@ -51,7 +51,7 @@ pub trait PortView {
 
     /// Iterates over all the input ports of the `node`.
     ///
-    /// Shorthand for [`PortView::ports`].
+    /// Shorthand for [`PortView::ports`]`(`[`Direction::Incoming`]`)`.
     #[must_use]
     #[inline]
     fn inputs(&self, node: NodeIndex) -> impl Iterator<Item = PortIndex> + Clone {
@@ -60,7 +60,7 @@ pub trait PortView {
 
     /// Iterates over all the output ports of the `node`.
     ///
-    /// Shorthand for [`PortView::ports`].
+    /// Shorthand for [`PortView::ports`]`(`[`Direction::Outgoing`]`)`.
     #[must_use]
     #[inline]
     fn outputs(&self, node: NodeIndex) -> impl Iterator<Item = PortIndex> + Clone {
@@ -81,7 +81,7 @@ pub trait PortView {
 
     /// Returns the number of input ports of the `node`.
     ///
-    /// Shorthand for [`PortView::num_ports`].
+    /// Shorthand for [`PortView::num_ports`]`(`[`Direction::Incoming`]`)`..
     #[must_use]
     #[inline]
     fn num_inputs(&self, node: NodeIndex) -> usize {
@@ -90,7 +90,7 @@ pub trait PortView {
 
     /// Returns the number of output ports of the `node`.
     ///
-    /// Shorthand for [`PortView::num_ports`].
+    /// Shorthand for [`PortView::num_ports`]`(`[`Direction::Outgoing`]`)`..
     #[must_use]
     #[inline]
     fn num_outputs(&self, node: NodeIndex) -> usize {
@@ -126,7 +126,7 @@ pub trait PortView {
 
     /// Iterates over all the input port offsets of the `node`.
     ///
-    /// Shorthand for [`PortView::port_offsets`].
+    /// Shorthand for [`PortView::port_offsets`]`(`[`Direction::Incoming`]`)`.
     #[must_use]
     #[inline]
     fn input_offsets(&self, node: NodeIndex) -> impl Iterator<Item = PortOffset> + Clone {
@@ -135,7 +135,7 @@ pub trait PortView {
 
     /// Iterates over all the output port offsets of the `node`.
     ///
-    /// Shorthand for [`PortView::port_offsets`].
+    /// Shorthand for [`PortView::port_offsets`]`(`[`Direction::Outgoing`]`)`.
     #[must_use]
     #[inline]
     fn output_offsets(&self, node: NodeIndex) -> impl Iterator<Item = PortOffset> + Clone {
@@ -283,7 +283,7 @@ pub trait LinkView: PortView {
     type LinkEndpoint: Into<PortIndex> + Copy;
 
     /// Returns an iterator over every pair of matching ports connecting `from`
-    /// with `to`.
+    /// to `to`, i.e. in that direction.
     ///
     /// # Example
     /// ```
@@ -350,7 +350,7 @@ pub trait LinkView: PortView {
         self.get_connection(from, to).is_some()
     }
 
-    /// Returns the port that the given `port` is linked to.
+    /// Returns the port (or ports if this is a [`MultiView`]) that the given `port` is linked to.
     #[must_use]
     fn port_links(
         &self,
@@ -401,7 +401,7 @@ pub trait LinkView: PortView {
     ) -> impl Iterator<Item = (Self::LinkEndpoint, Self::LinkEndpoint)> + Clone;
 
     /// Iterates over the connected input links of the `node`. Shorthand for
-    /// [`LinkView::links`].
+    /// [`LinkView::links`]`(`[`Direction::Incoming`]`)`.
     #[must_use]
     #[inline]
     fn input_links(
@@ -412,7 +412,7 @@ pub trait LinkView: PortView {
     }
 
     /// Iterates over the connected output links of the `node`. Shorthand for
-    /// [`LinkView::links`].
+    /// [`LinkView::links`]`(`[`Direction::Outgoing`]`)`..
     #[must_use]
     #[inline]
     fn output_links(
@@ -452,14 +452,18 @@ pub trait LinkView: PortView {
     #[must_use]
     fn all_neighbours(&self, node: NodeIndex) -> impl Iterator<Item = NodeIndex> + Clone;
 
-    /// Iterates over the input neighbours of the `node`. Shorthand for [`LinkView::neighbours`].
+    /// Iterates over the input neighbours of the `node`.
+    ///
+    /// Shorthand for [`LinkView::neighbours`]`(`[`Direction::Incoming`]`)`.
     #[must_use]
     #[inline]
     fn input_neighbours(&self, node: NodeIndex) -> impl Iterator<Item = NodeIndex> + Clone {
         self.neighbours(node, Direction::Incoming)
     }
 
-    /// Iterates over the output neighbours of the `node`. Shorthand for [`LinkView::neighbours`].
+    /// Iterates over the output neighbours of the `node`.
+    ///
+    /// Shorthand for [`LinkView::neighbours`]`(`[`Direction::Outgoing`]`)`.
     #[must_use]
     #[inline]
     fn output_neighbours(&self, node: NodeIndex) -> impl Iterator<Item = NodeIndex> + Clone {

src/view/subgraph.rs

@@ -40,8 +40,11 @@ use crate::{
 /// wall. The [Direction] of edges (incoming/outgoing) defines which side of
 /// the wall is inside, and which is outside.
 ///
-/// If both incoming and outgoing boundary edges are empty, the subgraph is
-/// taken to be the entire graph.
+/// Note that if the graph contains multiple connected components, there may be
+/// components none of whose edges are in the boundary. The definition above is
+/// consistent with such a component being either in or outside the subgraph.
+/// The [Self::new_subgraph] constructor offers only limited flexibility in
+/// defining such subgraphs and [Self::with_nodes] may need to be used instead.
 ///
 /// If an invalid subgraph is defined, then behaviour is undefined.
 ///
@@ -67,6 +70,13 @@ impl<G: LinkView> Subgraph<G> {
     ///   and outgoing ports are outgoing boundary edges.
     ///
     /// This initialisation is linear in the size of the subgraph.
+    ///
+    /// If both incoming and outgoing boundary edges are empty, the subgraph is taken
+    /// to be the entire graph (i.e. all components); otherwise, the subgraph
+    /// contains only (parts of) those components mentioned in the boundary.
+    /// (Specifically, where the boundary includes at least one edge, or disconnected
+    /// port. To create a subgraph containing the whole of a component without any
+    /// disconnected ports, use [`Self::with_nodes`].)
     pub fn new_subgraph(graph: G, boundary: Boundary) -> Self {
         let nodes = boundary.internal_nodes(&graph).collect();
         Self {
@@ -78,8 +88,8 @@ impl<G: LinkView> Subgraph<G> {
 
     /// Create a new subgraph of `graph` containing only the given nodes.
     ///
-    /// The boundary of the subgraph is defined by all ports of the given nodes,
-    /// in the order they are given.
+    /// The boundary of the subgraph is defined by all ports of the given nodes
+    /// that have edges to other (i.e. external) nodes, in the order `nodes` are given.
     ///
     /// See [`Subgraph::new_subgraph`] for a method that takes an explicit port boundary.
     pub fn with_nodes(graph: G, nodes: impl IntoIterator<Item = NodeIndex>) -> Self {
@@ -707,6 +717,52 @@ mod tests {
         assert!(!subg.is_convex());
     }
 
+    #[test]
+    fn test_disconnected_components() {
+        let mut graph = PortGraph::new();
+        let n0 = graph.add_node(0, 1);
+        let n1 = graph.add_node(1, 0);
+        graph.link_nodes(n0, 0, n1, 0).unwrap();
+        let n2 = graph.add_node(0, 1);
+        let n3 = graph.add_node(1, 1);
+        graph.link_nodes(n2, 0, n3, 0).unwrap();
+
+        let subg_nodes = |b| Subgraph::new_subgraph(&graph, b).nodes_iter().collect_vec();
+
+        // No edges -> all components
+        let b = Boundary::from_ports(&graph, []);
+        let nodes = subg_nodes(b.clone());
+        assert_eq!(nodes, [n0, n1, n2, n3]);
+        assert_eq!(Subgraph::with_nodes(&graph, nodes).boundary, b);
+
+        // Edge in only one component -> just (part of) that component
+        let b = Boundary::from_ports(&graph, [graph.output(n0, 0).unwrap()]);
+        let nodes = subg_nodes(b.clone());
+        assert_eq!(nodes, [n0]);
+        assert_eq!(Subgraph::with_nodes(&graph, nodes).boundary, b);
+
+        // Edges in two components -> relevant parts of each component
+        let b = Boundary::from_ports(
+            &graph,
+            [graph.output(n0, 0).unwrap(), graph.input(n3, 0).unwrap()],
+        );
+        let nodes = subg_nodes(b.clone());
+        assert_eq!(nodes, [n0, n3]);
+        assert_eq!(Subgraph::with_nodes(&graph, nodes).boundary, b);
+
+        // Disconnected port -> includes whole component:
+        let b = Boundary::from_ports(&graph, [graph.output(n3, 0).unwrap()]);
+        assert_eq!(subg_nodes(b), [n2, n3]);
+        // However the graph with those nodes includes the unconnected port as interior,
+        // so has an empty boundary:
+        assert_eq!(
+            Subgraph::with_nodes(&graph, [n2, n3]).boundary,
+            Boundary::from_ports(&graph, [])
+        );
+        // (The subgraph cannot be recreated from said boundary,
+        //    see https://github.com/CQCL/portgraph/issues/179.)
+    }
+
     #[test]
     fn test_copy_in_parent() {
         let mut graph = PortGraph::new();