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 bug in m2rotaxis #1171

Closed
wants to merge 10 commits into from

Conversation

RasmusFonseca
Copy link
Contributor

Takes 180deg rotation into account and added some documentation and tests.

@peterjc
Copy link
Member

peterjc commented Apr 19, 2017

Can I merge #494 from @bertrand-caron first?

Note there are some minor style issues to address, pip install flake8 should let you check this locally:

$ flake8 Bio/
Bio/PDB/Vector.py:17:85: W291 trailing whitespace
Bio/PDB/Vector.py:18:74: W291 trailing whitespace
Bio/PDB/Vector.py:22:5: E265 block comment should start with '# '
Bio/PDB/Vector.py:23:9: E201 whitespace after '('
Bio/PDB/Vector.py:23:17: E231 missing whitespace after ','
Bio/PDB/Vector.py:23:20: E226 missing whitespace around arithmetic operator
Bio/PDB/Vector.py:23:24: E231 missing whitespace after ','
Bio/PDB/Vector.py:23:38: W291 trailing whitespace
Bio/PDB/Vector.py:24:17: E231 missing whitespace after ','
Bio/PDB/Vector.py:24:20: E226 missing whitespace around arithmetic operator
Bio/PDB/Vector.py:24:24: E231 missing whitespace after ','
Bio/PDB/Vector.py:24:38: W291 trailing whitespace
Bio/PDB/Vector.py:25:17: E231 missing whitespace after ','
Bio/PDB/Vector.py:25:20: E226 missing whitespace around arithmetic operator
Bio/PDB/Vector.py:25:24: E231 missing whitespace after ','
Bio/PDB/Vector.py:25:35: E202 whitespace before ')'
Bio/PDB/Vector.py:26:9: E265 block comment should start with '# '
Bio/PDB/Vector.py:27:13: E201 whitespace after '('
Bio/PDB/Vector.py:27:21: E231 missing whitespace after ','
Bio/PDB/Vector.py:27:24: E226 missing whitespace around arithmetic operator
Bio/PDB/Vector.py:27:28: E231 missing whitespace after ','
Bio/PDB/Vector.py:27:42: W291 trailing whitespace
Bio/PDB/Vector.py:28:21: E231 missing whitespace after ','
Bio/PDB/Vector.py:28:24: E226 missing whitespace around arithmetic operator
Bio/PDB/Vector.py:28:28: E231 missing whitespace after ','
Bio/PDB/Vector.py:28:42: W291 trailing whitespace
Bio/PDB/Vector.py:29:21: E231 missing whitespace after ','
Bio/PDB/Vector.py:29:24: E226 missing whitespace around arithmetic operator
Bio/PDB/Vector.py:29:28: E231 missing whitespace after ','
Bio/PDB/Vector.py:30:21: E231 missing whitespace after ','
Bio/PDB/Vector.py:30:24: E226 missing whitespace around arithmetic operator
Bio/PDB/Vector.py:30:28: E231 missing whitespace after ','
Bio/PDB/Vector.py:30:31: E226 missing whitespace around arithmetic operator
Bio/PDB/Vector.py:30:35: E231 missing whitespace after ','
Bio/PDB/Vector.py:30:38: E226 missing whitespace around arithmetic operator
Bio/PDB/Vector.py:30:47: E202 whitespace before ')'
Bio/PDB/Vector.py:309:37: W291 trailing whitespace

@@ -285,11 +305,20 @@ def normsq(self):
return abs(sum(self._ar * self._ar))

def normalize(self):
"""Normalize the Vector."""
"""
Normalize the Vector object.
Copy link
Member

Choose a reason for hiding this comment

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

Please put the one-line docstring summary on the same line as the opening triple quote, as per examples in PEP257 https://www.python.org/dev/peps/pep-0257/#multi-line-docstrings

@RasmusFonseca
Copy link
Contributor Author

@peterjc Yes, you can merge his first, but there will be a minor conflict that needs to be resolved in the beginning of rotmat. Nothing big.

Thanks for pointers on style guide. Ill go through it at some point today.

@codecov
Copy link

codecov bot commented Apr 19, 2017

Codecov Report

Merging #1171 into master will increase coverage by 0.02%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1171      +/-   ##
==========================================
+ Coverage   79.72%   79.75%   +0.02%     
==========================================
  Files         323      323              
  Lines       51452    51454       +2     
==========================================
+ Hits        41020    41035      +15     
+ Misses      10432    10419      -13
Impacted Files Coverage Δ
Bio/PDB/Vector.py 88.62% <100%> (+9.83%) ⬆️
Bio/GA/Evolver.py 54.54% <0%> (-18.19%) ⬇️
Bio/Restriction/Restriction.py 52.27% <0%> (-0.12%) ⬇️
Bio/NeuralNetwork/Gene/Schema.py 71.9% <0%> (+0.82%) ⬆️

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 891f696...e05cf2a. Read the comment docs.

@peterjc
Copy link
Member

peterjc commented Apr 19, 2017

OK, I manually applied #494 - the merge conflict was due to a minor docstring change.

Do you want to rebase this branch against the new master, or should I do this?

@RasmusFonseca
Copy link
Contributor Author

It'd be great if you could. I haven't rebased before, and looking it up indicates there are enough steps that I could mess something up ;) Travis fails because I should have run flake8 Tests as well. Ill fix those first.

@peterjc
Copy link
Member

peterjc commented Apr 19, 2017

Sorry - this is a new style issue, TravisCI checks for this were enabled only earlier today:

$ flake8 Bio/
Bio/PDB/Vector.py:24:9: E129 visually indented line with same indent as next logical line
Bio/PDB/Vector.py:29:5: E129 visually indented line with same indent as next logical line

See 73896e4 - long lines might be an acceptable solution if you can't see how to break the long line nicely.

@RasmusFonseca
Copy link
Contributor Author

Haha. The wonders of trying to commit to an active project. I made them single line.

peterjc pushed a commit that referenced this pull request Apr 20, 2017
Fixes bug in m2rotaxis if rotation axis corresponds to 0 or 180 deg angle

Fixed bug where refmat alters the state of input-vectors

Split vector test into separate tests for rotmat and refmat,
and added more tests.

Squashed commit of pull request #1171.
peterjc added a commit that referenced this pull request Apr 20, 2017
See preceeding commit from pull request #1171.
@peterjc
Copy link
Member

peterjc commented Apr 20, 2017

I've manually applied this to the master branch, and included you in the acknowledgements. Thank you.

Apologies if I already asked, but are you happy to dual licence your contributions under both the Biopython License Agreement AND the BSD 3-Clause License? See #898.

@RasmusFonseca
Copy link
Contributor Author

You didn't ask, but I'm happy to appear as a contributor and I agree to the licenses.

@peterjc
Copy link
Member

peterjc commented Apr 20, 2017

Thank you

MarkusPiotrowski pushed a commit to MarkusPiotrowski/biopython that referenced this pull request Oct 31, 2017
Fixes bug in m2rotaxis if rotation axis corresponds to 0 or 180 deg angle

Fixed bug where refmat alters the state of input-vectors

Split vector test into separate tests for rotmat and refmat,
and added more tests.

Squashed commit of pull request biopython#1171.
MarkusPiotrowski pushed a commit to MarkusPiotrowski/biopython that referenced this pull request Oct 31, 2017
See preceeding commit from pull request biopython#1171.
peterjc added a commit to peterjc/biopython that referenced this pull request Aug 14, 2019
Original author Thomas Hamelryck (when the file was called
Bio/PDB/Vector.py) and all the contributors since as tracked
with version control have agreed:

 - Adhemar Zerlotini (@azneto)
   biopython#1412 (comment)
 - Bertrand Caron (@bertrand-caron)
   biopython#494 (comment)
 - Brian Osborne (@bosborne)
   biopython#898 (comment)
 - Christian Brueffer (@cbrueffer)
   biopython#898 (comment)
 - Brad Chapman (@chapmanb)
   http://mailman.open-bio.org/pipermail/biopython-dev/2013-July/019903.html
 - Connor McCoy (@cmccoy)
   biopython#898 (comment)
 - Eric Talevich (@etal)
   biopython#898 (comment)
 - Francesco Gastaldello (@Gasta88)
   biopython#1063
 - Hom (@platinhom)
   biopython#1920
 - Joe Greener (@jgreener64)
   biopython#1393
 - Michiel de Hoon (@mdehoon)
   biopython#898 (comment)
 - @morrme
   biopython#1172 (comment)
 - Peter Cock (@peterjc)
   biopython#898 (comment)
 - Ralf Stephan (@rwst)
   biopython#1900
 - Rasmus Fonseca (@fonseca.rasmus)
   biopython#1171 (comment)
 - Sergei Lebedev (@superbobry)
   biopython#898 (comment)
 - Sergio Valqui (@svalqui)
   biopython#1749
 - Thomas Hamelryck (CVS username nisse)
   https://mailman.open-bio.org/pipermail/biopython/2018-September/016546.html
peterjc added a commit that referenced this pull request Aug 29, 2019
Original author Thomas Hamelryck (when the file was called
Bio/PDB/Vector.py) and all the contributors since as tracked
with version control have agreed:

 - Adhemar Zerlotini (@azneto)
   #1412 (comment)
 - Bertrand Caron (@bertrand-caron)
   #494 (comment)
 - Brian Osborne (@bosborne)
   #898 (comment)
 - Christian Brueffer (@cbrueffer)
   #898 (comment)
 - Brad Chapman (@chapmanb)
   http://mailman.open-bio.org/pipermail/biopython-dev/2013-July/019903.html
 - Connor McCoy (@cmccoy)
   #898 (comment)
 - Eric Talevich (@etal)
   #898 (comment)
 - Francesco Gastaldello (@Gasta88)
   #1063
 - Hom (@platinhom)
   #1920
 - Joe Greener (@jgreener64)
   #1393
 - Michiel de Hoon (@mdehoon)
   #898 (comment)
 - @morrme
   #1172 (comment)
 - Peter Cock (@peterjc)
   #898 (comment)
 - Ralf Stephan (@rwst)
   #1900
 - Rasmus Fonseca (@fonseca.rasmus)
   #1171 (comment)
 - Sergei Lebedev (@superbobry)
   #898 (comment)
 - Sergio Valqui (@svalqui)
   #1749
 - Thomas Hamelryck (CVS username nisse)
   https://mailman.open-bio.org/pipermail/biopython/2018-September/016546.html
peterjc added a commit to peterjc/biopython that referenced this pull request Aug 30, 2019
Original authors Thomas Hamelryck and Eric Talevich (for the unittest
conversion), and the contributors as tracked with version control have
agreed:

 - Andrew Guy (@andrewguy)
   biopython#2212 (comment)
 - Ben Reynwar (@benreynwar)
   biopython#898 (comment)
 - Bernhard C. Thiel (@Bernhard10)
   biopython#968
 - Carlos Pena (@carlosp420)
   biopython#898 (comment)
 - Chris Rands (@chris-rands)
   biopython#1301
 - Christian Brueffer (@cbrueffer)
   biopython#898 (comment)
 - Eric Talevich (@etal)
   biopython#898 (comment)
 - Francesco Gastaldello (@Gasta88)
   biopython#1063
 - Gokcen Eraslan (@gokceneraslan)
   biopython#898 (comment)
 - Jack Twilley (@mathuin)
   biopython#1049 (comment)
 - Jeroen Van Goey (@BioGeek)
   biopython#898 (comment)
 - Joe Greener (@jgreener64)
   biopython#1393
 - Joerg Schaarschmidt (@schaarj)
   biopython#1567
 - João Rodrigues (@JoaoRodrigues)
   biopython#898 (comment)
 - Kian Ho (@kianho)
   biopython#898 (comment)
 - Kristian Davidsen (@krdav)
   biopython#898 (comment)
 - Kristian Rother (@krother)
   biopython#898 (comment)
 - Lenna Peterson (@lennax)
   biopython#898 (comment)
 - Markus Piotrowski (@MarkusPiotrowski)
   biopython#898 (comment)
 - Milind Luthra (@milindl)
   biopython#1020
 - Nick Negretti (@nimne)
   biopython#1768 (comment)
 - Peter Cock (@peterjc)
   biopython#898 (comment)
 - Rasmus Fonseca (@RasmusFonseca)
   biopython#1171 (comment)
 - Siong Kong (@siongkong)
   biopython#1516

 - Stefans Mezulis (@StefansM)
   biopython#1445
 - Thomas Hamelryck (CVS username nisse)
   https://mailman.open-bio.org/pipermail/biopython/2018-September/016546.html
peterjc added a commit that referenced this pull request Nov 4, 2019
Original authors Thomas Hamelryck and Eric Talevich (for the unittest
conversion), and the contributors as tracked with version control have
agreed:

 - Andrew Guy (@andrewguy)
   #2212 (comment)
 - Ben Reynwar (@benreynwar)
   #898 (comment)
 - Bernhard C. Thiel (@Bernhard10)
   #968
 - Carlos Pena (@carlosp420)
   #898 (comment)
 - Chris Rands (@chris-rands)
   #1301
 - Christian Brueffer (@cbrueffer)
   #898 (comment)
 - Eric Talevich (@etal)
   #898 (comment)
 - Francesco Gastaldello (@Gasta88)
   #1063
 - Gokcen Eraslan (@gokceneraslan)
   #898 (comment)
 - Jack Twilley (@mathuin)
   #1049 (comment)
 - Jeroen Van Goey (@BioGeek)
   #898 (comment)
 - Joe Greener (@jgreener64)
   #1393
 - Joerg Schaarschmidt (@schaarj)
   #1567
 - João Rodrigues (@JoaoRodrigues)
   #898 (comment)
 - Kian Ho (@kianho)
   #898 (comment)
 - Kristian Davidsen (@krdav)
   #898 (comment)
 - Kristian Rother (@krother)
   #898 (comment)
 - Lenna Peterson (@lennax)
   #898 (comment)
 - Markus Piotrowski (@MarkusPiotrowski)
   #898 (comment)
 - Milind Luthra (@milindl)
   #1020
 - Nick Negretti (@nimne)
   #1768 (comment)
 - Peter Cock (@peterjc)
   #898 (comment)
 - Rasmus Fonseca (@RasmusFonseca)
   #1171 (comment)
 - Siong Kong (@siongkong)
   #1516

 - Stefans Mezulis (@StefansM)
   #1445
 - Thomas Hamelryck (CVS username nisse)
   https://mailman.open-bio.org/pipermail/biopython/2018-September/016546.html
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.

None yet

2 participants