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.
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add substitute_node_with_subgraph method to PyDiGraph #312
Add substitute_node_with_subgraph method to PyDiGraph #312
Changes from 17 commits
058abc6
7cfdeb2
2ee3938
cfd8d0c
d03a4a6
1d45eec
427a943
e67e958
eafb002
f08a063
64606f2
5056244
23a0c42
dd08472
6dcbc85
681fcab
cca111b
4374d4a
80d1216
4ad41c7
411f6e9
ca7bbaa
e261144
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think
edge_map_fn
should be optional.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is required right now because we without it we don't know what to do with edges to or from
node
. If we switched it to optional what do you think the default behavior should it was not set? The only option I could think of is to just drop all the edges into or out ofnode
if this wasn't set, but if we did this it wouldn't really feel like it was substituting the node with a graph.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this a typo?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's actually intentional, the
/
indicates the end of positional only arguments (it's part of the cPython interface and is actually a valid python syntax for >=3.8). See the docs for the rust python interface here: https://pyo3.rs/v0.9.2/function.html#making-the-function-signature-available-to-python which explains this.