From efd9fc5fa40a808da0bf2095581983442b2b32c9 Mon Sep 17 00:00:00 2001 From: Richard Gowers Date: Mon, 12 Jun 2017 14:47:13 +0100 Subject: [PATCH 1/4] Aesthetic fixes to test_gro Removed TestCase usage --- .../MDAnalysisTests/coordinates/test_gro.py | 30 ++++++++++++------- 1 file changed, 20 insertions(+), 10 deletions(-) diff --git a/testsuite/MDAnalysisTests/coordinates/test_gro.py b/testsuite/MDAnalysisTests/coordinates/test_gro.py index beb202bdac0..ca08e35a3a4 100644 --- a/testsuite/MDAnalysisTests/coordinates/test_gro.py +++ b/testsuite/MDAnalysisTests/coordinates/test_gro.py @@ -20,22 +20,32 @@ # J. Comput. Chem. 32 (2011), 2319--2327, doi:10.1002/jcc.21787 # from __future__ import absolute_import -from unittest import TestCase import MDAnalysis as mda import numpy as np from MDAnalysis.coordinates.GRO import GROReader, GROWriter from MDAnalysisTests import make_Universe -from MDAnalysisTests.coordinates.base import BaseReference, BaseReaderTest, BaseWriterTest, BaseTimestepTest +from MDAnalysisTests.coordinates.base import ( + BaseReference, BaseReaderTest, BaseWriterTest, BaseTimestepTest, +) from MDAnalysisTests.coordinates.reference import RefAdK -from MDAnalysisTests.datafiles import COORDINATES_GRO, COORDINATES_GRO_INCOMPLETE_VELOCITY, COORDINATES_GRO_BZ2, GRO, \ - GRO_large +from MDAnalysisTests.datafiles import ( + COORDINATES_GRO, + COORDINATES_GRO_INCOMPLETE_VELOCITY, + COORDINATES_GRO_BZ2, + GRO, + GRO_large, +) from nose.plugins.attrib import attr -from numpy.testing import (assert_almost_equal, ) -from numpy.testing import assert_array_almost_equal, dec, assert_equal, assert_raises - - -class TestGROReaderOld(TestCase, RefAdK): +from numpy.testing import ( + assert_almost_equal, + assert_array_almost_equal, + dec, + assert_equal, + assert_raises +) + +class TestGROReaderOld(RefAdK): def setUp(self): self.universe = mda.Universe(GRO) self.ts = self.universe.trajectory.ts @@ -88,7 +98,7 @@ def test_unitcell(self): err_msg="unit cell dimensions (rhombic dodecahedron)") -class TestGROReaderNoConversionOld(TestCase, RefAdK): +class TestGROReaderNoConversionOld(RefAdK): def setUp(self): self.universe = mda.Universe(GRO, convert_units=False) self.ts = self.universe.trajectory.ts From 6a5fa2ce17e8747f4e72b35f8e762a545ac2377f Mon Sep 17 00:00:00 2001 From: Richard Gowers Date: Mon, 12 Jun 2017 15:30:08 +0100 Subject: [PATCH 2/4] Added tests for issue #1395 --- testsuite/MDAnalysisTests/coordinates/test_gro.py | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/testsuite/MDAnalysisTests/coordinates/test_gro.py b/testsuite/MDAnalysisTests/coordinates/test_gro.py index ca08e35a3a4..e240ad3aa21 100644 --- a/testsuite/MDAnalysisTests/coordinates/test_gro.py +++ b/testsuite/MDAnalysisTests/coordinates/test_gro.py @@ -24,7 +24,7 @@ import MDAnalysis as mda import numpy as np from MDAnalysis.coordinates.GRO import GROReader, GROWriter -from MDAnalysisTests import make_Universe +from MDAnalysisTests import make_Universe, tempdir from MDAnalysisTests.coordinates.base import ( BaseReference, BaseReaderTest, BaseWriterTest, BaseTimestepTest, ) @@ -38,6 +38,7 @@ ) from nose.plugins.attrib import attr from numpy.testing import ( + assert_, assert_almost_equal, assert_array_almost_equal, dec, @@ -417,6 +418,18 @@ def test_writer_large_residue_count(self): err_msg="Writing GRO file with > 99 999 " "resids does not truncate properly.") +@tempdir.run_in_tempdir() +def test_growriter_resid_truncation(): + u = make_Universe(extras=['resids'], trajectory=True) + u.residues[0].resid = 123456789 + u.atoms.write('out.gro') + + with open('out.gro', 'r') as grofile: + grofile.readline() + grofile.readline() + line = grofile.readline() + # larger digits should get truncated + assert_(line.startswith('56789UNK')) class TestGROTimestep(BaseTimestepTest): Timestep = mda.coordinates.GRO.Timestep From 930bb6349fde760eaa315a747a914aa469185f55 Mon Sep 17 00:00:00 2001 From: Richard Gowers Date: Mon, 12 Jun 2017 16:03:51 +0100 Subject: [PATCH 3/4] Added lib.util.ltruncate_integer --- package/MDAnalysis/lib/util.py | 25 ++++++++++++++++++++++ testsuite/MDAnalysisTests/lib/test_util.py | 16 ++++++++++++++ 2 files changed, 41 insertions(+) diff --git a/package/MDAnalysis/lib/util.py b/package/MDAnalysis/lib/util.py index c40c15be7b0..d75517c7553 100644 --- a/package/MDAnalysis/lib/util.py +++ b/package/MDAnalysis/lib/util.py @@ -1574,3 +1574,28 @@ def __eq__(self, other): except AssertionError: return False return True + + +def ltruncate_int(value, ndigits): + """Truncate an integer, retaining least significant digits + + Parameters + ---------- + value : int + value to truncate + ndigits : int + number of digits to keep + + Returns + ------- + truncated : int + only the `ndigits` least significant digits from `value` + + Examples + -------- + >>> ltruncate_int(123, 2) + 23 + >>> ltruncate_int(1234, 5) + 1234 + """ + return int(str(value)[-ndigits:]) diff --git a/testsuite/MDAnalysisTests/lib/test_util.py b/testsuite/MDAnalysisTests/lib/test_util.py index 1db65566a7d..ddf4558f884 100644 --- a/testsuite/MDAnalysisTests/lib/test_util.py +++ b/testsuite/MDAnalysisTests/lib/test_util.py @@ -968,3 +968,19 @@ def test_iter(self): seen.append(val) for val in ['this', 'that', 'other']: assert_(val in seen) + + +class TestTruncateInteger(object): + @staticmethod + def _check_vals(a, b): + assert_(util.ltruncate_int(*a) == b) + + def test_ltruncate_int(self): + for vals, exp in ( + ((1234, 1), 4), + ((1234, 2), 34), + ((1234, 3), 234), + ((1234, 4), 1234), + ((1234, 5), 1234), + ): + yield self._check_vals, vals, exp From abd205b8f8d4814d647f3f0062b7f8958cd416bb Mon Sep 17 00:00:00 2001 From: Richard Gowers Date: Mon, 12 Jun 2017 17:11:09 +0100 Subject: [PATCH 4/4] Changed coordinate writers to use ltruncate_int Fixes issue #1395 --- package/CHANGELOG | 5 +++-- package/MDAnalysis/coordinates/CRD.py | 6 +++--- package/MDAnalysis/coordinates/GRO.py | 4 ++-- package/MDAnalysis/coordinates/PDB.py | 4 ++-- package/MDAnalysis/coordinates/PDBQT.py | 4 ++-- 5 files changed, 12 insertions(+), 11 deletions(-) diff --git a/package/CHANGELOG b/package/CHANGELOG index d91b3b49d86..d3d96542a51 100644 --- a/package/CHANGELOG +++ b/package/CHANGELOG @@ -13,13 +13,14 @@ The rules for this file: * release numbers follow "Semantic Versioning" http://semver.org ------------------------------------------------------------------------------ -mm/dd/yy +mm/dd/17 - * 0.16.2 + * 0.16.2 richardjgowers Enhancements Fixes + * fixed GROWriter truncating long resids from the wrong end (Issue #1395) Changes diff --git a/package/MDAnalysis/coordinates/CRD.py b/package/MDAnalysis/coordinates/CRD.py index 70e7b857575..c005da25e8f 100644 --- a/package/MDAnalysis/coordinates/CRD.py +++ b/package/MDAnalysis/coordinates/CRD.py @@ -251,9 +251,9 @@ def write(self, selection, frame=None): current_resid += 1 # Truncate numbers - serial = int(str(i + 1)[-serial_len:]) - resid = int(str(resid)[-resid_len:]) - current_resid = int(str(current_resid)[-totres_len:]) + serial = util.ltruncate_int(i + 1, serial_len) + resid = util.ltruncate_int(resid, resid_len) + current_resid = util.ltruncate_int(current_resid, totres_len) crd.write(at_fmt.format( serial=serial, totRes=current_resid, resname=resname, diff --git a/package/MDAnalysis/coordinates/GRO.py b/package/MDAnalysis/coordinates/GRO.py index 3a36fa69d2f..7691b9fb2cb 100644 --- a/package/MDAnalysis/coordinates/GRO.py +++ b/package/MDAnalysis/coordinates/GRO.py @@ -377,8 +377,8 @@ def write(self, obj): # all attributes could be infinite cycles! for atom_index, resid, resname, name in zip( range(ag_or_ts.n_atoms), resids, resnames, names): - truncated_atom_index = int(str(atom_index + 1)[-5:]) - truncated_resid = int(str(resid)[:5]) + truncated_atom_index = util.ltruncate_int(atom_index + 1, 5) + truncated_resid = util.ltruncate_int(resid, 5) if has_velocities: output_gro.write(self.fmt['xyz_v'].format( resid=truncated_resid, diff --git a/package/MDAnalysis/coordinates/PDB.py b/package/MDAnalysis/coordinates/PDB.py index 2ac9b899f3f..910b42ca939 100644 --- a/package/MDAnalysis/coordinates/PDB.py +++ b/package/MDAnalysis/coordinates/PDB.py @@ -902,12 +902,12 @@ def get_attr(attrname, default): for i, atom in enumerate(atoms): vals = {} - vals['serial'] = int(str(i + 1)[-5:]) # check for overflow here? + vals['serial'] = util.ltruncate_int(i + 1, 5) # check for overflow here? vals['name'] = self._deduce_PDB_atom_name(atomnames[i], resnames[i]) vals['altLoc'] = altlocs[i][:1] vals['resName'] = resnames[i][:4] vals['chainID'] = segids[i][:1] - vals['resSeq'] = int(str(resids[i])[-4:]) + vals['resSeq'] = util.ltruncate_int(resids[i], 4) vals['iCode'] = icodes[i][:1] vals['pos'] = pos[i] # don't take off atom so conversion works vals['occupancy'] = occupancies[i] diff --git a/package/MDAnalysis/coordinates/PDBQT.py b/package/MDAnalysis/coordinates/PDBQT.py index 1cc2d4d3657..e16e2792831 100644 --- a/package/MDAnalysis/coordinates/PDBQT.py +++ b/package/MDAnalysis/coordinates/PDBQT.py @@ -320,12 +320,12 @@ def write(self, selection, frame=None): attrs['resids'], attrs['icodes'], attrs['occupancies'], attrs['tempfactors'], attrs['charges'], attrs['types']), start=1): - serial = int(str(serial)[-5:]) # check for overflow here? + serial = util.ltruncate_int(serial, 5) # check for overflow here? + resid = util.ltruncate_int(resid, 4) name = name[:4] if len(name) < 4: name = " " + name # customary to start in column 14 chainid = chainid.strip()[-1:] # take the last character - resid = int(str(resid)[-4:]) # check for overflow here? self.pdb.write(self.fmt['ATOM'].format( serial=serial,