Skip to content

Conversation

@liamhuber
Copy link
Member

Enforce the exclusive connection of nodes with the same (or None) parent.

This was always the intent. In order facilitate passing connections (but not a parent=) at instantiation time, when connections are made from a parented node to an orphan node, the orphan gets its counterpart's parent. This results in one gross side effect: making connections from an un-parented node to a node inside a macro after the macro has finished instantiating adds the un-parented node as a child of the macro! This has no functional impact since the execution graph of the macro is not updated post-instantiation, so that node just sits there taking up memory, but it is still sub-optimal.

Closes #587

Signed-off-by: liamhuber <liamhuber@greyhavensolutions.com>
Signed-off-by: liamhuber <liamhuber@greyhavensolutions.com>
Signed-off-by: liamhuber <liamhuber@greyhavensolutions.com>
Signed-off-by: liamhuber <liamhuber@greyhavensolutions.com>
Signed-off-by: liamhuber <liamhuber@greyhavensolutions.com>
Signed-off-by: liamhuber <liamhuber@greyhavensolutions.com>
@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@github-actions
Copy link

Binder 👈 Launch a binder notebook on branch pyiron/pyiron_workflow/intra_graph_connections

@codecov
Copy link

codecov bot commented Aug 13, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.23%. Comparing base (99f0472) to head (5b4853b).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #744      +/-   ##
==========================================
- Coverage   93.33%   93.23%   -0.11%     
==========================================
  Files          34       34              
  Lines        3586     3606      +20     
==========================================
+ Hits         3347     3362      +15     
- Misses        239      244       +5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@liamhuber
Copy link
Member Author

The missing coverage is actually relevant; before merging I should extend the test to include such a callback-triggering failure case.

@liamhuber
Copy link
Member Author

Also let's replace the callback class with with contextlib.ExitStack()

@liamhuber liamhuber marked this pull request as draft August 14, 2025 00:27
@liamhuber
Copy link
Member Author

liamhuber commented Aug 14, 2025

Also let's replace the callback class with with contextlib.ExitStack()

No, ExitStack triggers all its callback whenever we exit the stack context; I need something that only triggers callbacks when we hit an exception inside the context.

I could still use a context manager, but for now I only have a single exception anyway. It might be interesting to replace all such cleanup tasks with an exception-cleanup-manager, but that can be a separate PR.

EDIT: Better than here, if it's worth doing it's probably worth doing in snippets pyiron/pyiron_snippets#59

Signed-off-by: liamhuber <liamhuber@greyhavensolutions.com>
@liamhuber liamhuber marked this pull request as ready for review August 14, 2025 03:54
@liamhuber liamhuber merged commit e5d5ffc into main Aug 14, 2025
21 checks passed
@liamhuber liamhuber deleted the intra_graph_connections branch August 14, 2025 03:54
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.

No scope-breaking connections, please

2 participants