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

Fix for bug with mbuild missing edges in bond graph #1049

Merged
merged 2 commits into from
Aug 8, 2022

Conversation

CalCraven
Copy link
Contributor

PR Summary:

PR Checklist


  • Includes appropriate unit test(s)
  • Appropriate docstring(s) are added/updated
  • Code is (approximately) PEP8 compliant
  • Issue(s) raised/addressed?

@CalCraven
Copy link
Contributor Author

Fixes Issue #1048

@codecov
Copy link

codecov bot commented Aug 2, 2022

Codecov Report

Merging #1049 (21dc2a4) into main (85acbcf) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main    #1049   +/-   ##
=======================================
  Coverage   90.39%   90.39%           
=======================================
  Files          64       64           
  Lines        9027     9027           
=======================================
  Hits         8160     8160           
  Misses        867      867           
Impacted Files Coverage Δ
mbuild/bond_graph.py 89.13% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 85acbcf...21dc2a4. Read the comment docs.

Copy link
Member

@daico007 daico007 left a comment

Choose a reason for hiding this comment

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

LGTM!

@@ -147,7 +147,7 @@ def compose(self, graph):
adj = self._adj
for node, neighbors in graph._adj.items():
if self.has_node(node):
(adj[node].add(neighbor) for neighbor in neighbors)
[adj[node].add(neighbor) for neighbor in neighbors]
Copy link
Member

Choose a reason for hiding this comment

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

It's interesting how this could make such a big different.

Copy link
Contributor

@chrisjonesBSU chrisjonesBSU left a comment

Choose a reason for hiding this comment

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

LGTM

@daico007 daico007 merged commit 0970f90 into mosdef-hub:main Aug 8, 2022
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 this pull request may close these issues.

3 participants