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

Add atom index to Group()._repr_png_ #1758

Merged
merged 2 commits into from
Oct 31, 2019
Merged

Add atom index to Group()._repr_png_ #1758

merged 2 commits into from
Oct 31, 2019

Conversation

hwpang
Copy link
Contributor

@hwpang hwpang commented Oct 9, 2019

Description of Changes

Add atom index consistent with adjacency list to molecule group graph for users' convenience.

@hwpang hwpang requested a review from mliu49 October 9, 2019 22:41
@mliu49
Copy link
Contributor

mliu49 commented Oct 9, 2019

Out of curiosity, what happened with the other PR?

@hwpang
Copy link
Contributor Author

hwpang commented Oct 9, 2019

Out of curiosity, what happened with the other PR?

I was trying to use the PR function in VScode but things didn't work the way I expected lol
Sorry for confusion.

@mliu49
Copy link
Contributor

mliu49 commented Oct 10, 2019

There is a unit test for the Group.draw method which you will need to update.

@hwpang
Copy link
Contributor Author

hwpang commented Oct 10, 2019

There is a unit test for the Group.draw method which you will need to update.

unit test expected result modified

@codecov
Copy link

codecov bot commented Oct 10, 2019

Codecov Report

Merging #1758 into master will not change coverage.
The diff coverage is 0%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1758   +/-   ##
=======================================
  Coverage   32.61%   32.61%           
=======================================
  Files          87       87           
  Lines       26124    26124           
  Branches     6878     6878           
=======================================
  Hits         8521     8521           
+ Misses      16644    16633   -11     
- Partials      959      970   +11
Impacted Files Coverage Δ
rmgpy/molecule/group.py 0% <0%> (ø) ⬆️
rmgpy/data/statmech.py 42.2% <0%> (ø) ⬆️
rmgpy/rmg/pdep.py 12.21% <0%> (ø) ⬆️
rmgpy/reaction.py 0% <0%> (ø) ⬆️
rmgpy/data/kinetics/family.py 48.35% <0%> (ø) ⬆️
rmgpy/yml.py 15.71% <0%> (ø) ⬆️
rmgpy/data/kinetics/database.py 50.61% <0%> (ø) ⬆️
rmgpy/rmg/input.py 34% <0%> (ø) ⬆️
rmgpy/molecule/molecule.py 0% <0%> (ø) ⬆️
rmgpy/statmech/ndTorsions.py 59.78% <0%> (ø) ⬆️

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 6c016f6...ed87ab0. Read the comment docs.

@mliu49
Copy link
Contributor

mliu49 commented Oct 11, 2019

Could you rebase?

@hwpang hwpang force-pushed the moleculeGroupGraphIndex branch from d4ad98f to 1a23fd3 Compare October 11, 2019 01:03
@hwpang
Copy link
Contributor Author

hwpang commented Oct 11, 2019

Could you rebase?

Rebase is finished. @mliu49 :-)

@mliu49
Copy link
Contributor

mliu49 commented Oct 30, 2019

Sorry, I forgot to merge this. Could you rebase again?

@hwpang hwpang force-pushed the moleculeGroupGraphIndex branch from 1a23fd3 to 4fc5739 Compare October 30, 2019 18:59
@hwpang
Copy link
Contributor Author

hwpang commented Oct 30, 2019

Sorry, I forgot to merge this. Could you rebase again?

I rebased again. Thanks.

@hwpang hwpang force-pushed the moleculeGroupGraphIndex branch from 4fc5739 to ed87ab0 Compare October 30, 2019 22:54
@mliu49 mliu49 merged commit 0385f3a into master Oct 31, 2019
@mliu49 mliu49 deleted the moleculeGroupGraphIndex branch October 31, 2019 01:50
@mliu49 mliu49 mentioned this pull request Dec 16, 2019
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.

2 participants