[Solvergraph] add dynamically typed version of set_edges - #2158
Draft
tdavidcl wants to merge 5 commits into
Draft
Conversation
tdavidcl
commented
Aug 22, 2026
tdavidcl
commented
Member
- Finalize API
- Testing methodology?
Nodes wire their edges through EXPAND_NODE_EDGES-generated typed setters that require exact shared_ptr<T> per slot, which isn't expressible from Python bindings. Generate an additional set_edges overload taking plain vectors of IEdge shared_ptr, dynamic_cast each slot to the type the node actually expects, and raise a clear std::invalid_argument naming the slot, edge name, and expected/actual type on any mismatch (missing edge, null edge, wrong type, or wrong edge count). Optional slots accept a null entry as "no value". Assisted-by: Claude Code
Building shammodels_sph surfaced the warning on typeid(*edge).name(); bind a reference first, the same workaround INode::print_node_info() already uses for the identical typeid(*edge) pattern. Assisted-by: Claude Code
Assisted-by: Claude Code
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
|
Thanks @tdavidcl for opening this PR! You can do multiple things directly here: Once the workflow completes a message will appear displaying informations related to the run. Also the PR gets automatically reviewed by gemini, you can: |
Split the generic vector-based set_edges() into a three-tier chain instead of duplicating the per-slot casting logic: set_edges(vector, vector) picks each slot in order and forwards to set_edges_from_edges(shared_ptr<IEdge>...), which casts each slot to its concrete type via __node_edge_cast_checked(_optional) and forwards to the existing typed set_edges(shared_ptr<T>...). set_edges_from_edges() needed a distinct name from set_edges(): for a node whose edge type already is IEdge (NodeFreeAlloc), the typed and untyped overloads would otherwise be identical and fail to compile. Vector slots are also bound to named local variables before the forwarding call, since argument evaluation order between call arguments is unspecified in C++ and the previous ro_idx++/rw_idx++ per-argument pattern relied on left-to-right order. Assisted-by: Claude Code
Assisted-by: Claude Code
Contributor
Workflow reportworkflow report corresponding to commit 6d96dd1 Light CI is enabled (the default for pull requests). This will only run the basic tests and not the full tests. Pre-commit check reportPre-commit check: ✅ Test pipeline can run. Clang-tidy diff reportNo relevant changes found. You should now go back to your normal life and enjoy a hopefully sunny day while waiting for the review. Doxygen diff with
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.