Skip to content

Adds unwrap keyword to Asphericity method#2290

Merged
richardjgowers merged 20 commits intoMDAnalysis:developfrom
NinadBhat:asphericity-unwrap
Jul 10, 2019
Merged

Adds unwrap keyword to Asphericity method#2290
richardjgowers merged 20 commits intoMDAnalysis:developfrom
NinadBhat:asphericity-unwrap

Conversation

@NinadBhat
Copy link
Contributor

@NinadBhat NinadBhat commented Jul 9, 2019

Fixes #2291

Changes made in this Pull Request:

  • Adds unwrap keyword to Asphericity method

PR Checklist

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

@NinadBhat
Copy link
Contributor Author

Will make changes to this once #2288 is merged

@codecov
Copy link

codecov bot commented Jul 9, 2019

Codecov Report

Merging #2290 into develop will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #2290      +/-   ##
===========================================
+ Coverage    89.66%   89.67%   +<.01%     
===========================================
  Files          172      172              
  Lines        21421    21427       +6     
  Branches      2792     2794       +2     
===========================================
+ Hits         19208    19214       +6     
  Misses        1616     1616              
  Partials       597      597
Impacted Files Coverage Δ
package/MDAnalysis/core/topologyattrs.py 97.8% <100%> (+0.01%) ⬆️

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...a5d8c97. Read the comment docs.

@warn_if_not_unique
def asphericity(group, pbc=None):
@check_pbc_and_unwrap
def asphericity(group, pbc=None, unwrap=None, compound='group'):
Copy link
Member

Choose a reason for hiding this comment

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

You're adding a compound kwarg here with no tests, it's probably best not to add it now

Copy link
Contributor Author

@NinadBhat NinadBhat Jul 10, 2019

Choose a reason for hiding this comment

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

I have added tests for compound now. Should I remove it?

@richardjgowers richardjgowers merged commit 6185a57 into MDAnalysis:develop Jul 10, 2019
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.

Add "unwrap" keyword to asphericity

2 participants