Skip to content
This repository was archived by the owner on Dec 20, 2024. It is now read-only.

fix: use update_graph without attrs_cfg instead register_edges#85

Merged
JPXKQX merged 3 commits intodevelopfrom
fix/udpate-graph
Nov 26, 2024
Merged

fix: use update_graph without attrs_cfg instead register_edges#85
JPXKQX merged 3 commits intodevelopfrom
fix/udpate-graph

Conversation

@JPXKQX
Copy link
Copy Markdown
Member

@JPXKQX JPXKQX commented Nov 25, 2024

This PR addresses the following issue:

Bug Description

When moving from a single edge builder to multiple edge builders, attribute normalization was moved to occur after all edge builders registered their nodes. Specifically, register_edges() was called for each edge builder, and register_attributes() was executed once after all edges were defined. This caused a problem: node builders extending update_graph (e.g., with type checking or additional functionality) were not being executed during graph creation.

Fix

To resolve this, update_graph(graph, attrs_config=None) is now executed for each edge builder, ensuring expected behaviour during graph creation.

@JPXKQX JPXKQX requested a review from fprill November 25, 2024 11:33
@JPXKQX JPXKQX self-assigned this Nov 25, 2024
Copy link
Copy Markdown
Contributor

@fprill fprill left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello Mario, this works perfectly now. Thanks!

(This PR resolves the AttributeError: 'ICONTopologicalProcessorEdges' object has no attribute 'icon_sub_graph' which previously occurred because update_graph had not been called for the edge_builder)

@JPXKQX JPXKQX merged commit 809efc5 into develop Nov 26, 2024
@JPXKQX JPXKQX deleted the fix/udpate-graph branch December 16, 2024 15:57
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants