Skip to content
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

compose() erases some MultiGraph edges #1619

Closed
clbarnes opened this issue Jun 23, 2015 · 1 comment
Closed

compose() erases some MultiGraph edges #1619

clbarnes opened this issue Jun 23, 2015 · 1 comment

Comments

@clbarnes
Copy link

If a user wants to compose two multigraphs, it is very likely that they want to use all of the edges present in both. In MultiGraphs, edges which share the same (source, target) pair are not the same. Currently, edges which share the same (source, target, key) tuple are treated the same: as keys are assigned in insertion order by default, rather than having anything to do with the data, the end user just sees that an arbitrary few of their edges have gone missing.

import networkx as nx

a, b = nx.MultiGraph(), nx.MultiGraph()
a.add_path([1,2])
b.add_path([1,2])

nx.compose(a,b).number_of_edges() == a.number_of_edges() + b.number_of_edges()
>>> False

The documentation states that the edge sets are unioned. If this edge set is hashed by (source, target) pair, then the function cannot be advertised as applicable to MultiGraphs, because it collapses all multiedges. If the edge set is hashed by (source, target, key), as it is currently, then there is unexpected and possibly arbitrary behaviour which is not well documented. The edge set should be hashed by a UUID for MultiGraphs (i.e. all edges are distinct), in order to reflect how these classes are actually going to be used.

@dschult
Copy link
Member

dschult commented Jun 23, 2015

I can imagine users being confused by this if they use MultiGraphs without setting keys (which many don't). On the other hand, changing it as you suggest could be confusing to users who do pay attention to the edge keys. At the very least a note should be included in the docs.

Perhaps there should be an argument to describe how to treat the edge keys.

@hagberg hagberg added this to the networkx-2.0 milestone Dec 27, 2015
dschult added a commit to dschult/networkx that referenced this issue Apr 22, 2016
Fixes networkx#1619 while leaving networkx#1654 to decide about whether edges
should have unique identifiers (UUID).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

3 participants