Skip to content

Import export #32

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

Merged
merged 85 commits into from
Mar 20, 2019
Merged

Import export #32

merged 85 commits into from
Mar 20, 2019

Conversation

cgallay
Copy link
Contributor

@cgallay cgallay commented Sep 21, 2018

Hi @mdeff ,
Here is the PR for the "issue31" So that we can talk directly about it. Let me know what's need to be improved or any other comment.

Charles Gallay added 17 commits August 6, 2018 13:41
Export from PyGSP to external libraries.
Work in progress, the exports methods haven't been tested yet
change tempoary implemented in @classmethods and still not tested. Futhermore some optimization could be done especialy on the import from graph tool
The test create a graph in pygsp, convert it into the lib and load it back
A random graph from the lib si created as well and loaded then exported again
The documentation for the import and export methods have been started
Work in progress. not tested but the 'gml' format has been implemented
@coveralls
Copy link

Coverage Status

Coverage decreased (-4.9%) to 77.547% when pulling 7847448 on cgallay:importExport into 6386637 on epfl-lts2:master.

@coveralls
Copy link

coveralls commented Sep 21, 2018

Coverage Status

Coverage increased (+0.4%) to 87.054% when pulling bea1a1f on cgallay:importExport into 78034de on epfl-lts2:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-4.7%) to 77.68% when pulling 7847448 on cgallay:importExport into 6386637 on epfl-lts2:master.

@mdeff mdeff self-requested a review September 24, 2018 14:27
Copy link
Collaborator

@mdeff mdeff left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your work @cgallay! :)

I like the implementation in general. I've played with it and it works.

In the end I like the classmethod for the from_* methods. At some point we may also transition the graph constructors to class methods, and move the Graph object down to the pygsp namespace, i.e., we'd use pygsp.Graph.from_* instead of pygsp.graphs.Graph.from_*.

Some general comments:

  • Travis CI fails with ModuleNotFoundError: No module named 'graph_tool', as graph-tool needs to be installed. It might however be painful to install on Travis. Maybe it's easier to start using conda for CI. Or is there a way to conditionally ignore some tests? Please look into it and advise.
  • Use intersphinx to refer to the documentation of networkx and graph-tool. Look how it is done for pyunlocbox. Configuration in doc/conf.py.
  • Add the new features to doc/history.rst.
  • In general, use make lint to check style. Or have a style checker / static analyzer in your text editor. There's many errors prior to your change. Ignore those, but fix yours.
  • In docstrings, have an Examples section that shows how to use the function. That's of great help to the users!
  • Trim white space. There should be no spaces at the end of lines or on empty lines.
  • When importing and exporting signals and adjacency matrices, be sure that the node ordering corresponds.

There's many inline comments. But don't worry, that's training! As you learn, there will be less of them for the next PRs. :)

@mdeff
Copy link
Collaborator

mdeff commented Oct 4, 2018

If you execute the below code, you can open graph_nx.graphml in gephi and it will show the below figure. That's awesome! :)

import numpy as np
import networkx as nx
import pygsp as pg

graph = pg.graphs.Logo()
graph.estimate_lmax()

DELTAS = [20, 30, 1090]
signal = np.zeros(graph.n_nodes)
signal[DELTAS] = 1
signal = pg.filters.Heat(graph, tau=50).filter(signal)

graph.set_signal(graph.coords[:, 0], 'x')
graph.set_signal(graph.coords[:, 1], 'y')
graph.set_signal(signal, 'diffusion')

graph_nx = graph.to_networkx()

nx.write_graphml(graph_nx, 'graph_nx.graphml')
#nx.write_gml(graph_nx, 'graph_nx.gml')
#nx.write_gexf(graph_nx, 'graph_nx.gexf')

2018-10-04-16 56 35

The export to gml and gexf does however not work. Can you investigate why?

  • nx.write_gml(graph, 'graph.gml') gives NetworkXError: 103 is not a string
  • nx.write_gexf(graph, 'graph.gexf') gives KeyError: <class 'numpy.int16'>

The three works with graph-tool, but the graphml seems broken.

graph_gt = graph.to_graphtool(directed=False)
graph_gt.save('graph_gt.gml')
graph_gt.save('graph_gt.graphml')
graph_gt.save('graph_gt.dot')

I would suggest to export with all combinations of library and format, then look with a text editor if the files are fine. Then open them in gephi and cytoscape and look if everything (number of nodes, edges, edge weight, node properties, directed/undirected) is fine.

Another observation: graph_gt.graphml contains edges in both direction (a waste for undirected graphs), whereas 'graph_nx.graphml` only specify one of the edges. Networkx seems to do the right thing here. Is there a way to fix the graph-tool export?

@mdeff
Copy link
Collaborator

mdeff commented Oct 4, 2018

Importing seems to work with networkx and graphml:

g = nx.read_graphml('graph_nx.graphml')
g = pg.graphs.Graph.from_networkx(g, singals_names=['x', 'y', 'diffusion'])
g.coords = np.stack([g.signals['x'], g.signals['y']]).T
g.plot_signal(g.signals['diffusion'])

For graph-tool, it works with graphml and gml, but not dot:

g = gt.load_graph('graph_gt.graphml')
g = gt.load_graph('graph_gt.gml')
# g = gt.load_graph('graph_gt.dot')
g = pg.graphs.Graph.from_graphtool(g, singals_names=['x', 'y', 'diffusion'])
g.coords = np.stack([g.signals['x'], g.signals['y']]).T
g.plot_signal(g.signals['diffusion'])

Loading graph_gt.dot raises RuntimeError: Wanted right bracket to end attribute list (token is "<identifier> '7.96389'").

Exporting with networkx and importing with graph-tool works (i.e., g = gt.load_graph('graph_nx.graphml'). But exporting with graph-tool and importing with networkx does not, g = nx.read_graphml('graph_gt.graphml') raises ValueError: could not convert string to float: '0x1.83d2501c48897p-7'. There is something fishy with graph-tool graphml export.

@mdeff mdeff merged commit bea1a1f into epfl-lts2:master Mar 20, 2019
@mdeff mdeff mentioned this pull request Mar 20, 2019
3 tasks
@mdeff
Copy link
Collaborator

mdeff commented Mar 20, 2019

The implementation has been cleaned up in #46. Thanks @cgallay!

Droxef pushed a commit to Droxef/pygsp that referenced this pull request Jul 26, 2019
Droxef pushed a commit to Droxef/pygsp that referenced this pull request Jul 26, 2019
Droxef pushed a commit to Droxef/pygsp that referenced this pull request Jul 26, 2019
Droxef pushed a commit to Droxef/pygsp that referenced this pull request Jul 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants