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 keywords to filter residues in Janin analysis #2899

Merged
merged 6 commits into from
Aug 13, 2020
Merged

Conversation

orbeckst
Copy link
Member

Fixes #2898

Changes made in this Pull Request:

  • filter CYS*
  • updated docs and plots
  • made plots prettier (degree symbol on ticks)

PR Checklist

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

- fix #2898
- filter CYSH (actually, CYS*)
- updated documentation with Henry Mull's Technical Report
- added Ramachandran reference
- plots have degree symbol on ticks
- updated example plots and made sure that they show what's stated in the
  caption
- test added
- update CHANGELOG
@pep8speaks
Copy link

pep8speaks commented Aug 11, 2020

Hello @orbeckst! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2020-08-12 16:07:20 UTC

@orbeckst orbeckst requested a review from lilyminium August 11, 2020 03:07
Copy link
Member

@lilyminium lilyminium left a comment

Choose a reason for hiding this comment

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

Looks good, thank you @orbeckst; just a missing '>' in a reference means that the docs don't build.

If you wanted to be ultimately flexible and accommodate ncAAs etc, you could pass in keywords instead of hardcoding, e.g. Janin.__init__(u, select_protein="protein", select_remove="resname ALA CYS* GLY PRO SER THR VAL") but that's up to you.

package/MDAnalysis/analysis/dihedrals.py Outdated Show resolved Hide resolved
package/MDAnalysis/analysis/dihedrals.py Outdated Show resolved Hide resolved
package/MDAnalysis/analysis/dihedrals.py Outdated Show resolved Hide resolved
package/MDAnalysis/analysis/dihedrals.py Outdated Show resolved Hide resolved
package/MDAnalysis/analysis/dihedrals.py Outdated Show resolved Hide resolved
package/MDAnalysis/analysis/dihedrals.py Outdated Show resolved Hide resolved
orbeckst and others added 3 commits August 11, 2020 09:26
FIX: Janin analysis now correctly only selects from the provided input atomgroup

Co-authored-by: Lily Wang <31115101+lilyminium@users.noreply.github.com>
@orbeckst
Copy link
Member Author

@lilyminium can I please leave it to you to handle the PR? Ping me if I need to do anything else. Thanks!

@codecov
Copy link

codecov bot commented Aug 12, 2020

Codecov Report

Merging #2899 into develop will decrease coverage by 0.03%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #2899      +/-   ##
===========================================
- Coverage    92.89%   92.85%   -0.04%     
===========================================
  Files          187      187              
  Lines        24587    24593       +6     
  Branches      3192     3186       -6     
===========================================
- Hits         22840    22837       -3     
- Misses        1701     1710       +9     
  Partials        46       46              
Impacted Files Coverage Δ
package/MDAnalysis/analysis/dihedrals.py 100.00% <100.00%> (ø)
package/MDAnalysis/due.py 50.00% <0.00%> (-25.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 a9e749c...acd3697. Read the comment docs.

@lilyminium
Copy link
Member

Looks good, thank you -- I'll merge now :-)

@lilyminium lilyminium changed the title fix Janin analysis Add keywords to filter residues in Janin analysis Aug 13, 2020
@lilyminium lilyminium merged commit e091633 into develop Aug 13, 2020
@lilyminium lilyminium deleted the fix-janin branch August 13, 2020 13:50
PicoCentauri pushed a commit to PicoCentauri/mdanalysis that referenced this pull request Mar 30, 2021
Fixes MDAnalysis#2898

 - add select_protein and select_remove keywords to Janin analysis
 - filter CYS* by default
 - updated docs and plots
 - made plots prettier (degree symbol on ticks)
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.

Janin analysis does not filter CYSH
4 participants