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

Revise/nx graph #439

Merged
merged 7 commits into from Jan 22, 2018
Merged

Revise/nx graph #439

merged 7 commits into from Jan 22, 2018

Conversation

uvchik
Copy link
Member

@uvchik uvchik commented Jan 17, 2018

Revision of the graph module

  • I could not see the advantage of using a solph model instead of the energy system as the solph model gets its information from the energy system. That's why I removed the option to pass a model instead of an energy system.
  • I added a label to the nodes which helps to show the description in external tools
  • I added the weight to edges if an energy system is passed
  • I added the option to write the graph to disc as an .graphml file.

Things we may want to discuss

  • Do you like the integrated "write" option or do want to leave it to the user?
  • At them moment no "weight" attribute is written if the weight is None. We also could add an empty attribute. I think both ways have advantages.

@uvchik uvchik added this to the v0.2.1 milestone Jan 17, 2018
@uvchik uvchik self-assigned this Jan 17, 2018
@uvchik uvchik requested review from simnh and jnnr January 17, 2018 17:38
@uvchik
Copy link
Member Author

uvchik commented Jan 18, 2018

I had to fix my PR, because I forgot that we are not allowed to change the API for v0.2.1.
The optimisation_model attribute will now raise a FutureWarning, but will still work.
Here are the Travis results: https://travis-ci.org/oemof/oemof/builds/330299021

@jnnr
Copy link
Member

jnnr commented Jan 18, 2018

Looks good and gives nice results. Thanks! I like the labeling and weights. The integrated write function can be useful sometimes. If not needed, one just does not pass a filename, so that's perfectly ok.

The docstring could be revised. Under "Parameters", "filename" appears twice. Should I fix it?

@uvchik
Copy link
Member Author

uvchik commented Jan 18, 2018

The docstring could be revised. Under "Parameters", "filename" appears twice. Should I fix it?

Are you sure you have the latest commit?

@jnnr
Copy link
Member

jnnr commented Jan 18, 2018

Somehow I didn't. Strange. But now its ok.

@uvchik uvchik requested a review from jnnr January 18, 2018 16:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants