Bugfix: Multi-indexing fails in a very specific case (#249)

* add failing test

* update indexing of the groupby output

* update CHANGELOG.md

* address PR comments: test naming, make assertions more clear

* address PR comments: add more to the assertion message

CHANGELOG.md

@@ -27,6 +27,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Fixed
 
+* Issue when adding edges to fill spaces left by removing other edges (failure in resolving multi indexing) [#248](https://github.com/arup-group/genet/issues/248)
 * Fixed generating standard outputs by highway tag which were broken after moving to storing additional attributes in short form [#217](https://github.com/arup-group/genet/pull/217)
 * Fixed summary report:
   * Intermodal Access/Egress reporting is more general (not expecting just car and bike mode access to PT) [#204](https://github.com/arup-group/genet/pull/204)

src/genet/core.py

@@ -1308,6 +1308,7 @@ def generate_unique_multi_idx(group):
                 df_links[df_links["id"].isin(clashing_multi_idxs)]
                 .groupby(["from", "to"])
                 .apply(generate_unique_multi_idx)
+                .set_index("id", drop=False)
             )
 
             # generate unique indices if not

tests/test_core_network.py

@@ -1938,6 +1938,29 @@ def test_adding_multiple_links_with_id_and_multi_idx_clashes(assert_semantically
     )
 
 
+def test_avoids_destructive_index_clashes_when_adding_replacement_links(mocker):
+    n = Network("epsg:27700")
+    for i in range(3):
+        n.add_link(str(i), 1, 2)
+    n.remove_link(str(1))
+    # this method will sometimes output the smallest multiindex and sometimes just the next
+    # one after the highest used one. We fix it's return value to 1, an available index
+    # inbetween which caused this issue: https://github.com/arup-group/genet/issues/248
+    # I'm mocking this method here and not at the top of this test because adding the initial links
+    # to the graph would be impacted by it
+    mocker.patch.object(nx.MultiDiGraph, "new_edge_key", return_value=1)
+    links_to_add = {f"new-{i}": {"id": f"new-{i}", "from": 1, "to": 2} for i in range(3)}
+
+    reindexing_dict, links_and_attribs = n.add_links(links_to_add)
+
+    assert (
+        len(list(n.links())) == 5
+    ), "Network does not have the expected number of links: 5 = (3 original - 1 removed) + 3 new links"
+    assert (
+        len({v["multi_edge_idx"] for v in n.link_id_mapping.values()}) == 5
+    ), "Some multi-indices are not unique, the number of multi-indeces should match the number of links"
+
+
 def test_adding_multiple_links_missing_some_from_nodes():
     n = Network("epsg:27700")
     with pytest.raises(RuntimeError) as error_info: