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

Python 3 compatible sorting of atoms in PDB Residues #1291

Merged
merged 4 commits into from Aug 2, 2017

Conversation

peterjc
Copy link
Member

@peterjc peterjc commented Jun 22, 2017

This should close #1189, Bio.PDB sorting not tested and Python 2 only.

Note this removes the (apparently unused) private ._sort() methods of all the objects, and updates the only public .sort() method which is on the Residue object - for which I have added a test.

We could define (Python 3 compatible) public .sort() methods for the Chain, Model and Structure based on the existing Python 2 only private sort methods - but are they useful?

@codecov
Copy link

codecov bot commented Jun 22, 2017

Codecov Report

Merging #1291 into master will increase coverage by 0.06%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1291      +/-   ##
==========================================
+ Coverage   84.41%   84.47%   +0.06%     
==========================================
  Files         319      319              
  Lines       48850    48820      -30     
==========================================
+ Hits        41238    41242       +4     
+ Misses       7612     7578      -34
Impacted Files Coverage Δ
Bio/PDB/Model.py 95.45% <ø> (+22.12%) ⬆️
Bio/PDB/Structure.py 95.65% <ø> (+3.65%) ⬆️
Bio/PDB/Chain.py 92.3% <ø> (+13.58%) ⬆️
Bio/PDB/Residue.py 86.9% <100%> (+19.19%) ⬆️
Bio/Graphics/BasicChromosome.py 94% <0%> (-0.32%) ⬇️

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 f23e201...4e80ffb. Read the comment docs.

@peterjc
Copy link
Member Author

peterjc commented Jul 5, 2017

@etal @JoaoRodrigues what do you think about this PDB sort change?

@etal
Copy link
Contributor

etal commented Jul 7, 2017

These changes all look good and safe to me.

It would be reasonable for Chain, Model, and Structure be able to sort their child entities by ID. I don't recall needed to do that back when I was working with protein structures, though, since the files from PDB normally have that done already.

@peterjc
Copy link
Member Author

peterjc commented Jul 7, 2017

I agree - but the PDB model details are not fresh enough in my mind to quickly implement sorting at the other levels.

@peterjc
Copy link
Member Author

peterjc commented Jul 7, 2017

I've rebased this to deal with the merge conflict... waiting for tests...

The Residue object has a public sort method which has
been updated to use Python 3 compatible sorting by key.

The other objects never exposed sorting as a public method.
This is useful as flake8 etc will take into account
differences in the language itself and report a
slightly different set of issues on Python 2 vs 3.
@peterjc
Copy link
Member Author

peterjc commented Aug 2, 2017

Rebased again, with the missing colon in the TravisCI file fixed.

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.

Bio.PDB sorting not tested and Python 2 only
2 participants