Bugfix/json preserves node gaps

src/digraph.rs

@@ -3007,6 +3007,7 @@ impl PyDiGraph {
             }
             (None, true) => self.graph.contract_nodes(nodes, obj, check_cycle)?,
         };
+        self.node_removed = true;
         Ok(res.index())
     }
 

src/graph.rs

@@ -1899,6 +1899,7 @@ impl PyGraph {
             }
             (None, true) => self.graph.contract_nodes(nodes, obj),
         };
+        self.node_removed = true;
         Ok(res.index())
     }
 

src/json/mod.rs

@@ -74,12 +74,18 @@ pub fn from_node_link_json_file<'py>(
     Ok(if graph.directed {
         let mut inner_graph: StablePyGraph<Directed> =
             StablePyGraph::with_capacity(graph.nodes.len(), graph.links.len());
-        node_link_data::parse_node_link_data(&py, graph, &mut inner_graph, node_attrs, edge_attrs)?;
+        let node_removed = node_link_data::parse_node_link_data(
+            &py,
+            graph,
+            &mut inner_graph,
+            node_attrs,
+            edge_attrs,
+        )?;
         digraph::PyDiGraph {
             graph: inner_graph,
             cycle_state: algo::DfsSpace::default(),
             check_cycle: false,
-            node_removed: false,
+            node_removed,
             multigraph,
             attrs,
         }
@@ -88,11 +94,17 @@ pub fn from_node_link_json_file<'py>(
     } else {
         let mut inner_graph: StablePyGraph<Undirected> =
             StablePyGraph::with_capacity(graph.nodes.len(), graph.links.len());
-        node_link_data::parse_node_link_data(&py, graph, &mut inner_graph, node_attrs, edge_attrs)?;
+        let node_removed = node_link_data::parse_node_link_data(
+            &py,
+            graph,
+            &mut inner_graph,
+            node_attrs,
+            edge_attrs,
+        )?;
 
         graph::PyGraph {
             graph: inner_graph,
-            node_removed: false,
+            node_removed,
             multigraph,
             attrs,
         }
@@ -150,12 +162,18 @@ pub fn parse_node_link_json<'py>(
     Ok(if graph.directed {
         let mut inner_graph: StablePyGraph<Directed> =
             StablePyGraph::with_capacity(graph.nodes.len(), graph.links.len());
-        node_link_data::parse_node_link_data(&py, graph, &mut inner_graph, node_attrs, edge_attrs)?;
+        let node_removed = node_link_data::parse_node_link_data(
+            &py,
+            graph,
+            &mut inner_graph,
+            node_attrs,
+            edge_attrs,
+        )?;
         digraph::PyDiGraph {
             graph: inner_graph,
             cycle_state: algo::DfsSpace::default(),
             check_cycle: false,
-            node_removed: false,
+            node_removed,
             multigraph,
             attrs,
         }
@@ -164,10 +182,16 @@ pub fn parse_node_link_json<'py>(
     } else {
         let mut inner_graph: StablePyGraph<Undirected> =
             StablePyGraph::with_capacity(graph.nodes.len(), graph.links.len());
-        node_link_data::parse_node_link_data(&py, graph, &mut inner_graph, node_attrs, edge_attrs)?;
+        let node_removed = node_link_data::parse_node_link_data(
+            &py,
+            graph,
+            &mut inner_graph,
+            node_attrs,
+            edge_attrs,
+        )?;
         graph::PyGraph {
             graph: inner_graph,
-            node_removed: false,
+            node_removed,
             multigraph,
             attrs,
         }

src/json/node_link_data.rs

@@ -83,22 +83,70 @@ pub fn parse_node_link_data<Ty: EdgeType>(
     out_graph: &mut StablePyGraph<Ty>,
     node_attrs: Option<PyObject>,
     edge_attrs: Option<PyObject>,
-) -> PyResult<()> {
+) -> PyResult<bool> {
     let mut id_mapping: HashMap<usize, NodeIndex> = HashMap::with_capacity(graph.nodes.len());
-    for node in graph.nodes {
-        let payload = match node.data {
-            Some(data) => match node_attrs {
-                Some(ref callback) => callback.call1(*py, (data,))?,
-                None => data.into_py_any(*py)?,
-            },
-            None => py.None(),
-        };
-        let id = out_graph.add_node(payload);
-        match node.id {
-            Some(input_id) => id_mapping.insert(input_id, id),
-            None => id_mapping.insert(id.index(), id),
-        };
-    }
+
+    // Check if nodes have explicit IDs that need preservation
+    let preserve_ids = graph.nodes.iter().any(|n| n.id.is_some());
+
+    let node_removed = if preserve_ids {
+        // Find the maximum node ID to determine how many placeholder nodes we need
+        let max_id = graph.nodes.iter().filter_map(|n| n.id).max().unwrap_or(0);
+
+        // Create placeholder nodes up to max_id
+        let mut tmp_nodes: Vec<NodeIndex> = Vec::new();
+        for _ in 0..=max_id {
+            let idx = out_graph.add_node(py.None());
+            tmp_nodes.push(idx);
+        }
+
+        // Replace placeholder nodes with actual data and track which to keep
+        for node in graph.nodes {
+            let payload = match node.data {
+                Some(data) => match node_attrs {
+                    Some(ref callback) => callback.call1(*py, (data,))?,
+                    None => data.into_py_any(*py)?,
+                },
+                None => py.None(),
+            };
+            let node_id = node.id.unwrap_or(0);
+            let idx = NodeIndex::new(node_id);
+
+            // Replace the placeholder with actual data
+            if let Some(weight) = out_graph.node_weight_mut(idx) {
+                *weight = payload;
+            }
+
+            id_mapping.insert(node_id, idx);
+            // Mark this index as used (remove from tmp_nodes)
+            tmp_nodes.retain(|&n| n != idx);
+        }
+
+        // Track if we're removing any nodes (indicates gaps in indices)
+        let has_gaps = !tmp_nodes.is_empty();
+
+        // Remove remaining placeholder nodes
+        for tmp_node in tmp_nodes {
+            out_graph.remove_node(tmp_node);
+        }
+
+        has_gaps
+    } else {
+        // No explicit IDs, just add nodes sequentially (legacy behavior)
+        for node in graph.nodes {
+            let payload = match node.data {
+                Some(data) => match node_attrs {
+                    Some(ref callback) => callback.call1(*py, (data,))?,
+                    None => data.into_py_any(*py)?,
+                },
+                None => py.None(),
+            };
+            let id = out_graph.add_node(payload);
+            id_mapping.insert(id.index(), id);
+        }
+        false
+    };
+
     for edge in graph.links {
         let data = match edge.data {
             Some(data) => match edge_attrs {
@@ -109,7 +157,7 @@ pub fn parse_node_link_data<Ty: EdgeType>(
         };
         out_graph.add_edge(id_mapping[&edge.source], id_mapping[&edge.target], data);
     }
-    Ok(())
+    Ok(node_removed)
 }
 
 #[allow(clippy::too_many_arguments)]

tests/digraph/test_node_link_json.py

@@ -223,3 +223,45 @@ def test_not_JSON(self):
 """
         with self.assertRaises(rustworkx.JSONDeserializationError):
             rustworkx.parse_node_link_json(invalid_input)
+
+    def test_node_indices_preserved_with_deletion(self):
+        """Test that node indices are preserved after deletion (related to issue #1503)"""
+        graph = rustworkx.PyDiGraph()
+        graph.add_node("A")  # 0
+        graph.add_node("B")  # 1
+        graph.add_node("C")  # 2
+        graph.add_edge(0, 2, "A->C")
+        graph.remove_node(1)  # Remove middle node
+
+        # Verify original has gaps in indices
+        self.assertEqual([0, 2], graph.node_indices())
+
+        # Round-trip through JSON
+        json_str = rustworkx.node_link_json(graph)
+        restored = rustworkx.parse_node_link_json(json_str)
+
+        # Verify indices are preserved
+        self.assertEqual(graph.node_indices(), restored.node_indices())
+        self.assertEqual(graph.edge_list(), restored.edge_list())
+
+    def test_node_indices_preserved_with_contraction(self):
+        """Test that node indices are preserved after contraction (issue #1503)"""
+        graph = rustworkx.PyDiGraph()
+        graph.add_node("A")  # 0
+        graph.add_node("B")  # 1
+        graph.add_node("C")  # 2
+
+        # Contract nodes 0 and 1
+        contracted_idx = graph.contract_nodes([0, 1], "AB")
+        graph.add_edge(2, contracted_idx, "C->AB")
+
+        # Verify original has non-consecutive indices
+        self.assertEqual([2, contracted_idx], graph.node_indices())
+
+        # Round-trip through JSON
+        json_str = rustworkx.node_link_json(graph)
+        restored = rustworkx.parse_node_link_json(json_str)
+
+        # Verify indices are preserved
+        self.assertEqual(graph.node_indices(), restored.node_indices())
+        self.assertEqual(graph.edge_list(), restored.edge_list())

tests/digraph/test_pickle.py

@@ -39,3 +39,26 @@ def test_weight_graph(self):
         self.assertEqual([1, 2, 3], gprime.node_indices())
         self.assertEqual(["B", "C", "D"], gprime.nodes())
         self.assertEqual({1: (1, 2, "B -> C"), 3: (3, 1, "D -> B")}, dict(gprime.edge_index_map()))
+
+    def test_contracted_nodes_pickle(self):
+        """Test pickle/unpickle of directed graphs with contracted nodes (issue #1503)"""
+        g = rx.PyDiGraph()
+        g.add_node("A")  # Node 0
+        g.add_node("B")  # Node 1
+        g.add_node("C")  # Node 2
+
+        # Contract nodes 0 and 1 into a new node
+        contracted_idx = g.contract_nodes([0, 1], "AB")
+        g.add_edge(2, contracted_idx, "C -> AB")
+
+        # Verify initial state
+        self.assertEqual([2, contracted_idx], g.node_indices())
+        self.assertEqual([(2, contracted_idx)], g.edge_list())
+
+        # Test pickle/unpickle
+        gprime = pickle.loads(pickle.dumps(g))
+
+        # Verify the unpickled graph matches
+        self.assertEqual(g.node_indices(), gprime.node_indices())
+        self.assertEqual(g.edge_list(), gprime.edge_list())
+        self.assertEqual(g.nodes(), gprime.nodes())

tests/graph/test_node_link_json.py

@@ -204,3 +204,70 @@ def test_round_trip_with_file_no_graph_attr(self):
         self.assertEqual(new.nodes(), graph.nodes())
         self.assertEqual(new.weighted_edge_list(), graph.weighted_edge_list())
         self.assertEqual(new.attrs, {"label": graph.attrs})
+
+    def test_node_indices_preserved_with_deletion(self):
+        """Test that node indices are preserved after deletion (related to issue #1503)"""
+        graph = rustworkx.PyGraph()
+        graph.add_node(None)  # 0
+        graph.add_node(None)  # 1
+        graph.add_node(None)  # 2
+        graph.add_edge(0, 2, None)
+        graph.remove_node(1)  # Remove middle node
+
+        # Verify original has gaps in indices
+        self.assertEqual([0, 2], graph.node_indices())
+
+        # Round-trip through JSON
+        json_str = rustworkx.node_link_json(graph)
+        restored = rustworkx.parse_node_link_json(json_str)
+
+        # Verify indices are preserved
+        self.assertEqual(graph.node_indices(), restored.node_indices())
+        self.assertEqual(graph.edge_list(), restored.edge_list())
+
+    def test_node_indices_preserved_with_contraction(self):
+        """Test that node indices are preserved after contraction (issue #1503)"""
+        graph = rustworkx.PyGraph()
+        graph.add_node(None)  # 0
+        graph.add_node(None)  # 1
+        graph.add_node(None)  # 2
+
+        # Contract nodes 0 and 1
+        contracted_idx = graph.contract_nodes([0, 1], None)
+        graph.add_edge(2, contracted_idx, None)
+
+        # Verify original has non-consecutive indices
+        self.assertEqual([2, contracted_idx], graph.node_indices())
+
+        # Round-trip through JSON
+        json_str = rustworkx.node_link_json(graph)
+        restored = rustworkx.parse_node_link_json(json_str)
+
+        # Verify indices are preserved
+        self.assertEqual(graph.node_indices(), restored.node_indices())
+        self.assertEqual(graph.edge_list(), restored.edge_list())
+
+    def test_node_indices_preserved_complex(self):
+        """Test index preservation with multiple deletions and edges"""
+        graph = rustworkx.PyGraph()
+        for i in range(6):
+            graph.add_node(None)
+
+        graph.add_edge(0, 1, None)
+        graph.add_edge(2, 3, None)
+        graph.add_edge(4, 5, None)
+
+        # Remove nodes 1 and 4
+        graph.remove_node(1)
+        graph.remove_node(4)
+
+        # Verify gaps exist
+        self.assertEqual([0, 2, 3, 5], graph.node_indices())
+
+        # Round-trip through JSON
+        json_str = rustworkx.node_link_json(graph)
+        restored = rustworkx.parse_node_link_json(json_str)
+
+        # Verify complete state is preserved
+        self.assertEqual(graph.node_indices(), restored.node_indices())
+        self.assertEqual(graph.edge_list(), restored.edge_list())

tests/graph/test_pickle.py

@@ -39,3 +39,46 @@ def test_weight_graph(self):
         self.assertEqual([1, 2, 3], gprime.node_indices())
         self.assertEqual(["B", "C", "D"], gprime.nodes())
         self.assertEqual({1: (1, 2, "B -> C"), 3: (3, 1, "D -> B")}, dict(gprime.edge_index_map()))
+
+    def test_contracted_nodes_pickle(self):
+        """Test pickle/unpickle of graphs with contracted nodes (issue #1503)"""
+        g = rx.PyGraph()
+        g.add_node("A")  # Node 0
+        g.add_node("B")  # Node 1
+        g.add_node("C")  # Node 2
+
+        # Contract nodes 0 and 1 into a new node
+        contracted_idx = g.contract_nodes([0, 1], "AB")
+        g.add_edge(2, contracted_idx, "C -> AB")
+
+        # Verify initial state
+        self.assertEqual([2, contracted_idx], g.node_indices())
+        self.assertEqual([(2, contracted_idx)], g.edge_list())
+
+        # Test pickle/unpickle
+        gprime = pickle.loads(pickle.dumps(g))
+
+        # Verify the unpickled graph matches
+        self.assertEqual(g.node_indices(), gprime.node_indices())
+        self.assertEqual(g.edge_list(), gprime.edge_list())
+        self.assertEqual(g.nodes(), gprime.nodes())
+
+    def test_contracted_nodes_with_weights_pickle(self):
+        """Test pickle/unpickle of graphs with contracted nodes and edge weights"""
+        g = rx.PyGraph()
+        g.add_nodes_from(["Node0", "Node1", "Node2", "Node3"])
+        g.add_edges_from([(0, 2, "edge_0_2"), (1, 3, "edge_1_3")])
+
+        # Contract multiple nodes
+        contracted_idx = g.contract_nodes([0, 1], "Contracted_0_1")
+        g.add_edge(contracted_idx, 2, "contracted_to_2")
+        g.add_edge(3, contracted_idx, "3_to_contracted")
+
+        # Test pickle/unpickle
+        gprime = pickle.loads(pickle.dumps(g))
+
+        # Verify complete graph state is preserved
+        self.assertEqual(g.node_indices(), gprime.node_indices())
+        self.assertEqual(g.edge_list(), gprime.edge_list())
+        self.assertEqual(g.nodes(), gprime.nodes())
+        self.assertEqual(dict(g.edge_index_map()), dict(gprime.edge_index_map()))