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

[pyos][io] Conversion to NetworkX drops isolated vertices #475

Open
szhorvat opened this issue Jun 25, 2023 · 3 comments · May be fixed by #476
Open

[pyos][io] Conversion to NetworkX drops isolated vertices #475

szhorvat opened this issue Jun 25, 2023 · 3 comments · May be fixed by #476

Comments

@szhorvat
Copy link

szhorvat commented Jun 25, 2023

When converting to NetworkX, isolated vertices are dropped. Example:

M=gb.Matrix.from_edgelist([(0,1),(3,0)],nrows=5,ncols=5)
G=gb.io.to_networkx(M)

G.nodes gives NodeView((0, 1, 3)). Nodes 2 and 4 are both missing. This issue also affects viz.draw(). I do understand that it is one way to handle non-square matrices ...

Is this intentional or a bug? It is at least very surprising, and if intentional, it should be clearly documented.

Part of: pyOpenSci/software-submission#81

@SultanOrazbayev
Copy link
Member

SultanOrazbayev commented Jun 26, 2023

Thank you for raising this!

By creating isolates in networkx for empty rows/cols, we'd be making an assumption that such nodes exist. (edit: for example, in networkx, creating a graph by adding edges [(0,1), (3,0)] would not create nodes 2 and 4, though I understand this is not exactly apples to apples comparision here).

As such, this could be an additional convenience feature (as an additional kwarg such as create_isolates=True). Definitely something that can be documented.

SultanOrazbayev added a commit to SultanOrazbayev/python-graphblas that referenced this issue Jun 26, 2023
@szhorvat
Copy link
Author

szhorvat commented Jun 26, 2023

for example, in networkx, creating a graph by adding edges [(0,1), (3,0)] would not create nodes 2 and 4, though I understand this is not exactly apples to apples comparision here

Indeed these are not comparable concepts. In your example, 0, 1 and 3 are not vertex indices. They are just "names". They might as well be "foo", (2,3) and 3.14.

In comparison, in python-graphblas we have indices: 1st row (index 0), second row (index 1), etc.

The current behaviour is not treating the python-graphblas matrix as an adjacency matrix when converting to a graph. An n-vertex graph has n-by-n adjacency matrix. Instead, a rather idiosyncratic conversion method is used.

@eriknw
Copy link
Member

eriknw commented Jul 11, 2023

We discussed this and #476 in a recent meeting. I don't recall whether we got consensus, but there is an argument to be made that the nodes should always be added or at least be added by default. Thanks for raising this issue @szhorvat!

Converting GraphBLAS objects to and from graph objects from other libraries is something we'd like to tackle this year, so I think this issue is related to #398. Defining node attributes with a vector could also make sense.

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 a pull request may close this issue.

3 participants