Drop EntitySetView indexing by int id (#1187)

Graph type sets (`neo4j.graph.EntitySetView`) can no longer by indexed by
legacy `id` (`int`, e.g., `graph.nodes[0]`).

Use the `element_id` instead (`str`, e.g., `graph.nodes["..."]`).

Author: robsdedude

CHANGELOG.md

@@ -100,6 +100,8 @@ See also https://github.com/neo4j/neo4j-python-driver/wiki for a full changelog.
   Make sure to call `.close()` on them explicitly or use them in a `with` statement.
 - Make `Summary.summary_notifications` a `tuple` instead of a `list` and type it with `Sequence` to signify that it
   should be treated as immutable.
+- Graph type sets (`neo4j.graph.EntitySetView`) can no longer by indexed by legacy `id` (`int`, e.g., `graph.nodes[0]`).  
+  Use the `element_id` instead (`str`, e.g., `graph.nodes["..."]`).
 
 
 ## Version 5.28

src/neo4j/_codec/hydration/v1/hydration_handler.py

@@ -74,7 +74,6 @@ def hydrate_node(self, id_, labels=None, properties=None, element_id=None):
         except KeyError:
             inst = Node(self.graph, element_id, id_, labels, properties)
             self.graph._nodes[element_id] = inst
-            self.graph._legacy_nodes[id_] = inst
         else:
             # If we have already hydrated this node as the endpoint of
             # a relationship, it won't have any labels or properties.
@@ -125,7 +124,6 @@ def hydrate_unbound_relationship(
             r = self.graph.relationship_type(type_)
             inst = r(self.graph, element_id, id_, properties)
             self.graph._relationships[element_id] = inst
-            self.graph._legacy_relationships[id_] = inst
         return inst
 
     def hydrate_path(self, nodes, relationships, sequence):

src/neo4j/graph/__init__.py

@@ -21,10 +21,11 @@
 import typing as t
 from collections.abc import Mapping
 
-from .._warnings import (
-    deprecated,
-    deprecation_warn,
-)
+
+if t.TYPE_CHECKING:
+    from typing_extensions import deprecated
+else:
+    from .._warnings import deprecated
 
 
 __all__ = [
@@ -50,15 +51,10 @@ class Graph:
 
     def __init__(self) -> None:
         self._nodes: dict[str, Node] = {}
-        self._legacy_nodes: dict[int, Node] = {}  # TODO: 6.0 - remove
         self._relationships: dict[str, Relationship] = {}
-        # TODO: 6.0 - remove
-        self._legacy_relationships: dict[int, Relationship] = {}
         self._relationship_types: dict[str, type[Relationship]] = {}
-        self._node_set_view = EntitySetView(self._nodes, self._legacy_nodes)
-        self._relationship_set_view = EntitySetView(
-            self._relationships, self._legacy_relationships
-        )
+        self._node_set_view = EntitySetView(self._nodes)
+        self._relationship_set_view = EntitySetView(self._relationships)
 
     @property
     def nodes(self) -> EntitySetView[Node]:
@@ -209,19 +205,10 @@ class EntitySetView(Mapping, t.Generic[_T]):
     def __init__(
         self,
         entity_dict: dict[str, _T],
-        legacy_entity_dict: dict[int, _T],
     ) -> None:
         self._entity_dict = entity_dict
-        self._legacy_entity_dict = legacy_entity_dict  # TODO: 6.0 - remove
-
-    def __getitem__(self, e_id: int | str) -> _T:
-        # TODO: 6.0 - remove this compatibility shim
-        if isinstance(e_id, (int, float, complex)):
-            deprecation_warn(
-                "Accessing entities by an integer id is deprecated, "
-                "use the new style element_id (str) instead"
-            )
-            return self._legacy_entity_dict[e_id]
+
+    def __getitem__(self, e_id: str) -> _T:
         return self._entity_dict[e_id]
 
     def __len__(self) -> int:

tests/unit/async_/work/test_result.py

@@ -681,17 +681,9 @@ async def test_result_graph(records):
         nodes = graph.nodes
 
         assert set(nodes._entity_dict) == {"0", "1"}
-        for key in (
-            "0",
-            0,
-            0.0,
-            # I pray to god that no-one actually accessed nodes with complex
-            # numbers, but theoretically it would have worked with the legacy
-            # number IDs
-            0 + 0j,
-        ):
+        for key in ("0", 0, 0.0, 0 + 0j):
             if not isinstance(key, str):
-                with pytest.warns(DeprecationWarning, match="element_id"):
+                with pytest.raises(KeyError):
                     alice = nodes[key]
             else:
                 alice = nodes[key]
@@ -703,7 +695,7 @@ async def test_result_graph(records):
 
         for key in ("1", 1, 1.0, 1 + 0j):
             if not isinstance(key, str):
-                with pytest.warns(DeprecationWarning, match="element_id"):
+                with pytest.raises(KeyError):
                     bob = nodes[key]
             else:
                 bob = nodes[key]
@@ -720,7 +712,7 @@ async def test_result_graph(records):
 
         for key in ("0", 0, 0.0, 0 + 0j):
             if not isinstance(key, str):
-                with pytest.warns(DeprecationWarning, match="element_id"):
+                with pytest.raises(KeyError):
                     rel = rels[key]
             else:
                 rel = rels[key]

tests/unit/common/graph/test_graph.py

@@ -54,11 +54,7 @@ def with_relationship(
         start_node_element_id = f"e{start_node_id}"
         end_node_element_id = f"e{end_node_id}"
         self._relationship_counter += 1
-        with pytest.warns(DeprecationWarning):
-            assert start_node_id in self._hydrator.graph.nodes
         assert start_node_element_id in self._hydrator.graph.nodes
-        with pytest.warns(DeprecationWarning):
-            assert end_node_id in self._hydrator.graph.nodes
         assert end_node_element_id in self._hydrator.graph.nodes
 
         self._hydrator.hydrate_relationship(
@@ -363,3 +359,25 @@ def test_copy_path():
     graph1 = path1.graph
     graph2 = path2.graph
     assert graph1 is graph2
+
+
+def test_node_id_access() -> None:
+    assert 0 not in GRAPH.nodes
+    with pytest.raises(KeyError):
+        _ = GRAPH.nodes[0]  # type: ignore # expected to fail
+
+
+def test_node_element_id_access() -> None:
+    assert "e0" in GRAPH.nodes
+    _ = GRAPH.nodes["e0"]
+
+
+def test_relationship_id_access() -> None:
+    assert 0 not in GRAPH.relationships
+    with pytest.raises(KeyError):
+        _ = GRAPH.relationships[0]  # type: ignore # expected to fail
+
+
+def test_relationship_element_id_access() -> None:
+    assert "e0" in GRAPH.relationships
+    _ = GRAPH.relationships["e0"]

tests/unit/common/test_types.py

@@ -570,13 +570,13 @@ def test_graph_views_v1():
     g = hydration_scope.get_graph()
     assert len(g.nodes) == 3
     for id_, node in ((1, alice), (2, bob), (3, carol)):
-        with pytest.warns(DeprecationWarning, match=r"element_id \(str\)"):
+        with pytest.raises(KeyError):
             assert g.nodes[id_] == node
         assert g.nodes[str(id_)] == node
 
     assert len(g.relationships) == 2
     for id_, rel in ((1, alice_knows_bob), (2, carol_dislikes_bob)):
-        with pytest.warns(DeprecationWarning, match=r"element_id \(str\)"):
+        with pytest.raises(KeyError):
             assert g.relationships[id_] == rel
         assert g.relationships[str(id_)] == rel
 
@@ -668,24 +668,24 @@ def test_graph_views_v2_repr(example_graph_builder_v2, legacy_id):
         (2, g.bob.element_id, g.bob),
         (3, g.carol.element_id, g.carol),
     ):
-        with pytest.warns(DeprecationWarning, match=r"element_id \(str\)"):
-            assert g.graph.nodes[id_] == node
+        with pytest.raises(KeyError):
+            assert g.graph.nodes[id_] == node  # type: ignore # should fail
         assert g.graph.nodes[element_id] == node
         if not legacy_id:
             with pytest.raises(KeyError):
-                g.graph.nodes[str(id_)]
+                _ = g.graph.nodes[str(id_)]
 
     assert len(g.graph.relationships) == 2
     for id_, element_id, rel in (
         (1, g.alice_knows_bob.element_id, g.alice_knows_bob),
         (2, g.carol_dislikes_bob.element_id, g.carol_dislikes_bob),
     ):
-        with pytest.warns(DeprecationWarning, match=r"element_id \(str\)"):
-            assert g.graph.relationships[id_] == rel
+        with pytest.raises(KeyError):
+            assert g.graph.relationships[id_] == rel  # type: ignore
         assert g.graph.relationships[element_id] == rel
         if not legacy_id:
             with pytest.raises(KeyError):
-                g.graph.relationships[str(id_)]
+                _ = g.graph.relationships[str(id_)]
 
 
 @pytest.mark.parametrize("legacy_id", (True, False))