From 537d23a2a67cd001d5ce106d68ee24e0fe7667ee Mon Sep 17 00:00:00 2001 From: rhoadesScholar Date: Thu, 16 Jul 2026 05:20:57 +0000 Subject: [PATCH 1/2] Fix update_nodes for Vec (multi-column / array) node attributes update_nodes could not update a Vec node attribute: - SQLite stores a Vec across per-dimension columns (embedding_0, ...), but update_nodes wrote the whole sequence into a single column (`SET embedding = [1.0, 2.0, 3.0]`), which is invalid SQL. It now expands each attribute to its column(s) via _node_attrs_to_columns and emits one assignment per column (scalars unchanged; None handled). - PostgreSQL stores a Vec as a single native array column; __convert_to_sql now renders list/tuple/ndarray values as an array literal '{v0,v1,...}' (matching the COPY insert format), and normalizes numpy scalars/arrays. Adds a regression test parametrized over the sqlite/psql providers. --- .../persistence/graphs/sql_graph_database.py | 29 ++++++++++++++++--- tests/test_graph.py | 28 ++++++++++++++++++ 2 files changed, 53 insertions(+), 4 deletions(-) diff --git a/src/funlib/persistence/graphs/sql_graph_database.py b/src/funlib/persistence/graphs/sql_graph_database.py index 5cb511d..c3c9a71 100644 --- a/src/funlib/persistence/graphs/sql_graph_database.py +++ b/src/funlib/persistence/graphs/sql_graph_database.py @@ -704,10 +704,24 @@ def update_nodes( ) continue - values = [data.get(attr) for attr in attrs] - setters = [ - f"{k} = {self.__convert_to_sql(v)}" for k, v in zip(attrs, values) - ] + setters = [] + for attr in attrs: + value = data.get(attr) + columns = self._node_attrs_to_columns([attr]) + if len(columns) == 1: + setters.append(f"{columns[0]} = {self.__convert_to_sql(value)}") + else: + # attr is stored across multiple columns (e.g. a Vec in the + # SQLite backend -> attr_0, attr_1, ...); expand the value to + # one assignment per column instead of writing the whole + # sequence into a single column (which produced invalid SQL). + components = ( + [None] * len(columns) if value is None else list(value) + ) + setters.extend( + f"{col} = {self.__convert_to_sql(component)}" + for col, component in zip(columns, components) + ) update_statement = ( f"UPDATE {self.nodes_table_name} SET " f"{', '.join(setters)} WHERE " @@ -806,12 +820,19 @@ def __get_node_pos(self, n: dict[str, Any]) -> Optional[Coordinate]: return None def __convert_to_sql(self, x: Any) -> str: + if hasattr(x, "tolist") and not isinstance(x, (str, bytes)): + # normalize numpy scalars/arrays to native python + x = x.tolist() if isinstance(x, str): return f"'{x}'" elif x is None: return "null" elif isinstance(x, bool): return f"{x}".lower() + elif isinstance(x, (list, tuple)): + # a native array column (e.g. a Vec stored as a single array column + # in the PostgreSQL backend) -> array literal '{v0,v1,...}' + return "'{" + ",".join(map(str, x)) + "}'" else: return str(x) diff --git a/tests/test_graph.py b/tests/test_graph.py index eef1e4e..2a7c103 100644 --- a/tests/test_graph.py +++ b/tests/test_graph.py @@ -180,6 +180,34 @@ def test_graph_read_and_update_specific_attrs(provider_factory): assert data["c"] == 5 +def test_graph_update_vector_node_attr(provider_factory): + # Regression: update_nodes must handle a Vec (multi-column) node attribute, + # not just scalars. Previously the SQLite backend emitted + # `UPDATE nodes SET embedding = [1.0, 2.0, 3.0]` (invalid SQL) because it + # wrote the whole sequence into one column instead of expanding to the + # per-dimension columns (embedding_0, embedding_1, ...). + roi = Roi((0, 0, 0), (10, 10, 10)) + graph = nx.Graph() + graph.add_node(7, position=(5, 5, 5), embedding=[0.0, 0.0, 0.0]) + + with provider_factory( + "w", node_attrs={"position": Vec(float, 3), "embedding": Vec(float, 3)} + ) as gp: + gp.write_graph(graph) + + with provider_factory("r+") as gp: + g = gp.read_graph(roi) + g.nodes[7]["embedding"] = [1.0, 2.0, 3.0] + try: + gp.update_nodes(g.nodes, roi=roi, attributes=["embedding"]) + except NotImplementedError: + pytest.xfail() + + with provider_factory("r") as gp: + updated = gp.read_graph(roi) + assert list(updated.nodes[7]["embedding"]) == [1.0, 2.0, 3.0] + + def test_graph_read_unbounded_roi(provider_factory, write_method): unbounded_roi = Roi((None, None, None), (None, None, None)) graph = nx.Graph() From 0660c752c80f6554f20ea4f23a5c0f26730722bb Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 22 Jul 2026 17:46:26 +0000 Subject: [PATCH 2/2] Address review: explicit numpy checks in __convert_to_sql Replace the duck-typed hasattr(x, "tolist") check (and the redundant str/bytes exclusion) with explicit isinstance checks against np.ndarray and np.generic. Only numpy values are normalized to native python; every other datatype passes through exactly as it did before. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01DVCWK56VDzoEoCRwbkxBcg --- .../persistence/graphs/sql_graph_database.py | 12 +++++++----- tests/test_graph.py | 19 +++++++++++++++---- 2 files changed, 22 insertions(+), 9 deletions(-) diff --git a/src/funlib/persistence/graphs/sql_graph_database.py b/src/funlib/persistence/graphs/sql_graph_database.py index c3c9a71..e1c0811 100644 --- a/src/funlib/persistence/graphs/sql_graph_database.py +++ b/src/funlib/persistence/graphs/sql_graph_database.py @@ -2,6 +2,7 @@ from abc import abstractmethod from typing import Any, Iterable, Optional +import numpy as np from funlib.geometry import Coordinate, Roi from networkx import DiGraph, Graph from networkx.classes.reportviews import NodeView, OutEdgeView @@ -715,9 +716,7 @@ def update_nodes( # SQLite backend -> attr_0, attr_1, ...); expand the value to # one assignment per column instead of writing the whole # sequence into a single column (which produced invalid SQL). - components = ( - [None] * len(columns) if value is None else list(value) - ) + components = [None] * len(columns) if value is None else list(value) setters.extend( f"{col} = {self.__convert_to_sql(component)}" for col, component in zip(columns, components) @@ -820,9 +819,12 @@ def __get_node_pos(self, n: dict[str, Any]) -> Optional[Coordinate]: return None def __convert_to_sql(self, x: Any) -> str: - if hasattr(x, "tolist") and not isinstance(x, (str, bytes)): - # normalize numpy scalars/arrays to native python + # normalize numpy values to their native python equivalents; all + # other types pass through untouched + if isinstance(x, np.ndarray): x = x.tolist() + elif isinstance(x, np.generic): + x = x.item() if isinstance(x, str): return f"'{x}'" elif x is None: diff --git a/tests/test_graph.py b/tests/test_graph.py index 2a7c103..858dd76 100644 --- a/tests/test_graph.py +++ b/tests/test_graph.py @@ -1,4 +1,5 @@ import networkx as nx +import numpy as np import pytest from funlib.geometry import Roi @@ -188,24 +189,34 @@ def test_graph_update_vector_node_attr(provider_factory): # per-dimension columns (embedding_0, embedding_1, ...). roi = Roi((0, 0, 0), (10, 10, 10)) graph = nx.Graph() - graph.add_node(7, position=(5, 5, 5), embedding=[0.0, 0.0, 0.0]) + graph.add_node(7, position=(5, 5, 5), embedding=[0.0, 0.0, 0.0], score=0.0) with provider_factory( - "w", node_attrs={"position": Vec(float, 3), "embedding": Vec(float, 3)} + "w", + node_attrs={ + "position": Vec(float, 3), + "embedding": Vec(float, 3), + "score": float, + }, ) as gp: gp.write_graph(graph) with provider_factory("r+") as gp: g = gp.read_graph(roi) - g.nodes[7]["embedding"] = [1.0, 2.0, 3.0] + # numpy values (the typical case, e.g. embeddings computed with numpy) + # must be normalized to native python on their way into the SQL + # statement, for arrays and scalars alike + g.nodes[7]["embedding"] = np.array([1.0, 2.0, 3.0]) + g.nodes[7]["score"] = np.float64(0.5) try: - gp.update_nodes(g.nodes, roi=roi, attributes=["embedding"]) + gp.update_nodes(g.nodes, roi=roi, attributes=["embedding", "score"]) except NotImplementedError: pytest.xfail() with provider_factory("r") as gp: updated = gp.read_graph(roi) assert list(updated.nodes[7]["embedding"]) == [1.0, 2.0, 3.0] + assert updated.nodes[7]["score"] == 0.5 def test_graph_read_unbounded_roi(provider_factory, write_method):