Skip to content

fix(graphs): keep edge_weights/node_weights/coords in sync after add_edges_from - #445

Closed
agu2347 wants to merge 1 commit into
pasqal-io:mainfrom
agu2347:fix-add-edges-from-stale-weights
Closed

agu2347 wants to merge 1 commit into
pasqal-io:mainfrom
agu2347:fix-add-edges-from-stale-weights

Conversation

@agu2347

@agu2347 agu2347 commented Aug 12, 2026

Copy link
Copy Markdown

What

BaseGraph._reset_dicts() seeds _edge_weights (and _node_weights) from the graph's current nodes/edges, but is only ever called at construction time (or explicitly, by the alternative constructors). add_edges_from is inherited directly from nx.Graph and never routes through it, so edges (and any nodes they implicitly create) added after construction are silently missing from edge_weights, with no error or warning:

from qoolqit.graphs import DataGraph

g = DataGraph.from_nodes([0, 1, 2])
g.add_edges_from([(0, 1), (1, 2)])

print(g.edge_weights)  # {} -- expected {(0, 1): None, (1, 2): None}

This repo's own test suite was already routing around the bug -- test_to_matrix_unweighted called graph._reset_dicts() right after add_edges_from with a # FIXME: ... see issue #431 comment above it.

Fix

Override add_edges_from on BaseGraph (inherited by DataGraph, which doesn't define its own): call through to nx.Graph.add_edges_from, then seed any node/edge that's genuinely new with a weight/coordinate of None, matching _reset_dicts()'s own convention. Existing weights are left untouched, so this can't clobber edge_weights/node_weights set explicitly after construction.

New nodes are also handled (not just edges), since add_edges_from can implicitly introduce nodes that don't exist yet -- leaving those out of _node_weights/_coords would surface later as a KeyError in unrelated code paths (e.g. to_matrix(), distances()).

Testing

  • Removed the _reset_dicts() workaround (and its # FIXME) from test_to_matrix_unweighted.
  • Added test_add_edges_from_updates_weights_and_coords, covering: the exact case from the issue, that existing weights survive a later add_edges_from call, and that edges introducing brand-new nodes extend node_weights/coords for those nodes too.
  • Ran the full test_base_graph.py suite (47 tests): all pass except one pre-existing, unrelated failure (test_from_nx_wrong_pos_attr, a & vs and short-circuit bug in from_nx's validation, confirmed present on main before this change too).
  • Reverting just the base_graph.py change makes the new regression test fail with the exact symptom from the issue (edge_weights == {} instead of {(0, 1): None, (1, 2): None}).
  • ruff check / ruff format --check clean on both changed files.

Fixes #431

…edges_from

`BaseGraph._reset_dicts` seeds `_edge_weights` (and `_node_weights`) from
the graph's nodes/edges only at construction time. `add_edges_from` is
inherited directly from `nx.Graph` and bypasses `_reset_dicts`, so edges
(and any nodes they implicitly create) added after construction were
silently missing from `edge_weights`, with no error or warning:

    g = DataGraph.from_nodes([0, 1, 2])
    g.add_edges_from([(0, 1), (1, 2)])
    print(g.edge_weights)  # {} -- expected {(0, 1): None, (1, 2): None}

Override `add_edges_from` on `BaseGraph` to call through to NetworkX and
then seed any newly-added node/edge with a weight/coordinate of None
(matching `_reset_dicts`'s own convention), leaving existing weights
untouched. `DataGraph` inherits this fix automatically since it does not
override `add_edges_from` itself.

Also removes the `_reset_dicts()` workaround (with its `# FIXME: ... see
issue pasqal-io#431` comment) that `test_to_matrix_unweighted` needed to route
around the bug, and adds a dedicated regression test covering: the
exact case from the issue, that existing weights survive a later
`add_edges_from` call, and that edges introducing brand-new nodes extend
`node_weights`/`coords` for those nodes too (instead of leaving them
missing, which would otherwise surface later as a KeyError).

Fixes pasqal-io#431
@sgrava

sgrava commented Aug 13, 2026

Copy link
Copy Markdown
Collaborator

Hi @agu2347 ,
Thanks for proposing the bug fix! We really appreciate external inputs.
Unfortunately we were already fixing this in a separate PR #438 .

If you wish to contribute to the library, please consider other open issues.
Thanks.

@sgrava sgrava closed this Aug 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

edge_weights does not reflect edges added after construction

2 participants