fixed Issue #2001 - support for Tinker periodic boundary box#2011
fixed Issue #2001 - support for Tinker periodic boundary box#2011richardjgowers merged 7 commits intoMDAnalysis:developfrom
Conversation
kain88-de
left a comment
kain88-de
left a comment
There was a problem hiding this comment.
Thank you for the fix. There are some minor issues with the comments. It would be nice if you can address them.
| names = np.zeros(natoms, dtype=object) | ||
| types = np.zeros(natoms, dtype=np.int) | ||
| bonds = [] | ||
| #Checks whether file contains periodic boundary box info |
There was a problem hiding this comment.
The comment would be better if it explains why this works. Currently, it only explains what the code does. So something along the lines of
PBC box information is stored optionally in tinker files at the beginning of a frame. The box is stored as (x, y, z, alpha, beta, gamma). If not box information is stored the second value (space separated) is an atom name. To detect if a box is stored we, therefore, need to check if the second value can be converted to float. This assumes there are no pure numerical atom names.
it is longer but much better understandable for a maintainer later/
| @@ -43,6 +43,7 @@ | |||
| """ | |||
|
|
|||
| from __future__ import absolute_import | |||
There was a problem hiding this comment.
Since you now use zip we also need to import the iterating zip from six. Please add the line from six.moves import zip below the future import.
| self.xyzfile = util.anyopen(self.filename) | ||
| self._cache = dict() | ||
|
|
||
| with util.openany(self.filename) as inp: |
There was a problem hiding this comment.
also add a comment here why the code works.
| fline = inf.readline() | ||
| try: | ||
| float(fline.split()[1]) | ||
| except: |
| "XYZ", "XYZ_psf", "XYZ_bz2", | ||
| "XYZ_mini", "XYZ_five", # 3 and 5 atoms xyzs for an easy topology | ||
| "TXYZ", "ARC", # Tinker files | ||
| "TXYZ", "ARC", "ARC_PDB", # Tinker files |
There was a problem hiding this comment.
it's called ARC_PDB here but ARC_PBC everywhere else
|
There's not from __future__ import absolute_import, zipMakes Travis unhappy. |
|
Zip is in six.moves and not the future module.
…On Sat 4. Aug 2018 at 00:14, Oliver Beckstein ***@***.***> wrote:
There's not __future__.zip:
from __future__ import absolute_import, zip
Makes Travis unhappy.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#2011 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AEGnVlfuGUSAFsWVRCSLr2QbAbWOluVNks5uNMtOgaJpZM4Vk4Uk>
.
|
Codecov Report
@@ Coverage Diff @@
## develop #2011 +/- ##
===========================================
+ Coverage 88.5% 88.59% +0.08%
===========================================
Files 143 143
Lines 17249 17305 +56
Branches 2646 2649 +3
===========================================
+ Hits 15266 15331 +65
+ Misses 1385 1376 -9
Partials 598 598
Continue to review full report at Codecov.
|
| # Can't infinitely read as XYZ files can be multiframe | ||
| for i in range(natoms): | ||
| line = inf.readline().split() | ||
| for i, line in zip(range(natoms), itertools.chain([fline], inf)): |
There was a problem hiding this comment.
The linter was complaining about this line because it thought we weren't iterating the range, but we are, so I changed the linter RC file.
4 participants
Fixes #2001
Changes made in this Pull Request:
PR Checklist