Skip to content

Adds unwrap keyword to principal_axes #2295

Closed
NinadBhat wants to merge 15 commits intoMDAnalysis:developfrom
NinadBhat:principal_axes_unwrap
Closed

Adds unwrap keyword to principal_axes #2295
NinadBhat wants to merge 15 commits intoMDAnalysis:developfrom
NinadBhat:principal_axes_unwrap

Conversation

@NinadBhat
Copy link
Copy Markdown
Contributor

Fixes #

Changes made in this Pull Request:

PR Checklist

  • Tests?
  • Docs?
  • CHANGELOG updated?
  • Issue raised/referenced?

.. versionchanged:: 0.8 Added *pbc* keyword

"""
atomgroup = group.atoms
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@richardjgowers @jbarnoud Is there a reason for using atomgroup = group.atoms
in place of directly using group.moment_of_inertia?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Here it should be fine since MOI is doing the conversion. In general these methods are attached to all groups, so ResidueGroup too, but they want to operate on atoms. So for a RG, atomgroup = group.atoms is converting to an AtomGroup within the method.

@codecov
Copy link
Copy Markdown

codecov bot commented Jul 9, 2019

Codecov Report

Merging #2295 into develop will decrease coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #2295      +/-   ##
===========================================
- Coverage    89.66%   89.65%   -0.01%     
===========================================
  Files          172      172              
  Lines        21421    21421              
  Branches      2792     2792              
===========================================
- Hits         19208    19206       -2     
- Misses        1616     1617       +1     
- Partials       597      598       +1
Impacted Files Coverage Δ
package/MDAnalysis/core/topologyattrs.py 97.78% <100%> (ø) ⬆️
package/MDAnalysis/core/universe.py 94.42% <0%> (-0.56%) ⬇️

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 2f87852...a6bdacd. Read the comment docs.

@orbeckst
Copy link
Copy Markdown
Member

@NinadBhat @richardjgowers what is missing to merge this PR?

@NinadBhat
Copy link
Copy Markdown
Contributor Author

@orbeckst I was waiting for #2299 to be merged. Once that is merged I will merge develop into this PR and resolve the conflicts.
I am updating Projects Page, which contain the PRs that need to be reviewed and merged.

@orbeckst
Copy link
Copy Markdown
Member

Ok, thanks. Good idea to focus on the things are blocking others.

@IAlibay IAlibay added the close? Evaluate if issue/PR is stale and can be closed. label May 18, 2021
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

close? Evaluate if issue/PR is stale and can be closed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants