Relational compose #4329
Relational compose #4329
Conversation
…mposition of G and H as relations. Nodes of G; H are the set union of nodes of G and nodes of H. G; H contains an edge from A to C for every combination of an edge in G from A to B and an edge in H from B to C, for any node B. For multigraphs, with keys=True only edges with matching keys are acted upon.
…ompose. This should probably be allowed for other operators as well. Define to_multigraph and use to convert from simple graphs.
stevenganz
force-pushed
the
relational_compose
branch
from
3d526d8
to
f6796a5
Compare
|
You don't need the |
|
Regarding the formatting, I installed the commit hook (why does it require waiving security checks?) but only after I had put my commits in place. I tried merging them into a different branch, but that didn’t trigger any formatting. The formatting did look fine to me though, and seemed to match the existing style. Regarding to_multigraph, that isn’t technically necessary, and for that reason is in its an upper commit on its own (and can thus be easily dropped), but does make life much easier for the user to not have to worry about what kind of graph they have. I see no reason not to do use both to_directed and to_multigraph in preprocessing all operator arguments the way I do with relational_compose. MultiGraph(G) wasn’t working for me. I tried it in testing to_multigraph. I’ll try to reproduce that and open an issue. |
| graphs = iter(graphs) | ||
| C = next(graphs) | ||
| for H in graphs: | ||
| C = nx.relational_compose(C, H, with_keys) |
There was a problem hiding this comment.
Can you make a variant of relational compose such that it does an in-place update on C if possible by setting a kwarg copy=False? This would make this much faster for large numbers of graphs.
There was a problem hiding this comment.
I don't see why not. That does sound like it would be useful.
| R.add_nodes_from(H.nodes(data=True)) | ||
|
|
||
| # add composite edges to result graph | ||
| for (gsource, nbrdict) in G.adjacency_iter(): |
There was a problem hiding this comment.
You don't need a new adjacency_iter method here. You can just do
for gsource, nbrdict in G.adj.items():There was a problem hiding this comment.
OK, thanks. I see that G.adjacency_iter was replaced by G..adjacency, which is no longer supported. I was referring to some old documentation and also there are still stray references to adjacency_iter in the current version. No problem, I'll do it the way you suggest.
There was a problem hiding this comment.
Yeah, there used to be lots of different ways to do it. Since the the view overhaul there was work on consolidating things such that there was "one true way" to do everything. In this case the .adj attribute has everything you need.
There was a problem hiding this comment.
It's best to deprecate and then remove all traces of the previous ways.
There was a problem hiding this comment.
Where are the stray references to adjacency_iter? We got rid of that years ago.
There was a problem hiding this comment.
Dan, You pointed out the definition of adjacency below, and there are various calls to it. There's an adjacency_iter at examples/subclass/plot_antigraph.py which, although not directly applicable, led me to believe that defining such a function was the appropriate way to handle this.
There was a problem hiding this comment.
Thank you! I can believe that we missed that one... I'll open an issue. :}
|
|
||
| # add composite edges to result graph | ||
| for (gsource, nbrdict) in G.adjacency_iter(): | ||
| for gtarget in nbrdict.keys(): |
There was a problem hiding this comment.
using .keys is superfluous here, but its never bad to be explicit.
| # for each edge in H from gtarget to succ_node with the same key as the edge from gsource to gtarget in G, create an edge in R with that key pre-composing the edge from gsource to gtarget in G | ||
| for k in succ_nodes[succ_node]: | ||
| if k in G[gsource][gtarget]: | ||
| R.add_edge(gsource, succ_node, key=k, **(edge_data_combiner(G[gsource][gtarget][k], H[gtarget][succ_node][k]) if with_data and edge_data_combiner else {})) |
There was a problem hiding this comment.
probably best to separate the data dict rectification step into a previous line. It will make errors in edge_data_combiner easier to trace.
| @@ -355,6 +363,23 @@ def adj(self): | |||
| """ | |||
| return AdjacencyView(self._adj) | |||
|
|
|||
| def adjacency_iter(self): | |||
There was a problem hiding this comment.
not necessary, see above comment
There was a problem hiding this comment.
There is already a def adjacency(self): which does this for the Graph classes. ~ line 1370
There was a problem hiding this comment.
I see. Am I correct that G.adjacency is last documented in ver. 2.1?
| @@ -1,5 +1,6 @@ | |||
| import pytest | |||
| import networkx as nx | |||
| from networkx.algorithms.operators.all import relational_compose_all # Why can't this be accessed from nx.relational_compose_all? | |||
There was a problem hiding this comment.
Is it not exposed in the __init__.py? Is it missing from some __all__ registry?
There was a problem hiding this comment.
Ah, was left out of the __all__ registry. Thanks.
|
This has been stale for quite some time and will likely need significant TLC to get up and running again. Are you still planning to work on this @stevenganz ? |
Related (circuitously) to Issue #4208. Adds the relational_compose operator and updates to work with edge data attributes and to mix graph types.