search

RWTH-EBC / uesgraphs

Graph framework for urban energy systems
MIT License
23 stars 6 forks source link

adds a workaround to access nodes with node #18

Closed sebuer closed 4 years ago

sebuer commented 4 years ago

The current workaround in networkx was removed which enabled access of nodes with the node attribute. This commit adds the workaround in uesgraphs. fixes #17

coveralls commented 4 years ago

Pull Request Test Coverage Report for Build 513

Totals Coverage Status
Change from base Build 482: 0.0%
Covered Lines:
Relevant Lines: 0
💛 - Coveralls
coveralls commented 4 years ago

Pull Request Test Coverage Report for Build 513

Totals Coverage Status
Change from base Build 482: 0.0%
Covered Lines:
Relevant Lines: 0
💛 - Coveralls
MichaMans commented 4 years ago

@sebuer thanks for the fix and the PR. Did you check if this still works with older networkx versions e.g. 2.1? If yes, we can merge, if not, we should state this fix in the setup.py as a fixed requirement.

sebuer commented 4 years ago

@MichaMans with this change all unit tests are working for the networkx versions 2.1 to the current version 2.4. The issue could also be resolved by inserting node = nx.Graph.nodes instead of creating a property. I also tested this change for all networkx versions since 2.1 and could find no issue. In my opinion though this not as good for reliability then the change made in 8fcf0e9

MichaMans commented 4 years ago

@sebuer perfect! If this works with the other versions too, then I'll merge it and make a new release. Thank you very much!