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 Dihedral box #198

Merged
merged 19 commits into from
Sep 14, 2021
Merged

Fix Dihedral box #198

merged 19 commits into from
Sep 14, 2021

Conversation

ALescoulie
Copy link
Contributor

I added a box argument to calc_dihedrals similar to what is present in MDAnalysis

@ALescoulie ALescoulie requested a review from orbeckst September 14, 2021 21:15
@ALescoulie ALescoulie self-assigned this Sep 14, 2021
@ALescoulie ALescoulie added this to the 0.8.0 milestone Sep 14, 2021
@codecov
Copy link

codecov bot commented Sep 14, 2021

Codecov Report

Merging #198 (a191e82) into develop (5628473) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop     #198   +/-   ##
========================================
  Coverage    78.53%   78.53%           
========================================
  Files           11       11           
  Lines         1677     1677           
  Branches       250      250           
========================================
  Hits          1317     1317           
  Misses         276      276           
  Partials        84       84           
Impacted Files Coverage Δ
mdpow/analysis/dihedral.py 100.00% <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 5628473...a191e82. Read the comment docs.

Copy link
Member

@orbeckst orbeckst left a comment

Choose a reason for hiding this comment

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

I am going to approve without asking for the obligatory "could you please test this" because I know that it's tested in MDA — that's why using existing (trustworthy) libraries is good!

@orbeckst orbeckst merged commit 9810b74 into Becksteinlab:develop Sep 14, 2021
@orbeckst
Copy link
Member

I think you need to update your own branch because lot's of old commits are showing up. The squash merge just dealt with it but in the future it would be good to have clean PRs for review.

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