From 0e784b6a29ec7f4e5720246462c85c6ef1a43328 Mon Sep 17 00:00:00 2001 From: CalCraven Date: Fri, 7 Aug 2020 10:55:35 -0400 Subject: [PATCH 1/8] fix masses and charge bug --- package/MDAnalysis/topology/HoomdXMLParser.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/package/MDAnalysis/topology/HoomdXMLParser.py b/package/MDAnalysis/topology/HoomdXMLParser.py index 5a0f5da1aae..a0834c3305b 100644 --- a/package/MDAnalysis/topology/HoomdXMLParser.py +++ b/package/MDAnalysis/topology/HoomdXMLParser.py @@ -121,12 +121,11 @@ def parse(self, **kwargs): ): try: val = configuration.find(attrname) - vals = [mapper(el) for el in val.text.strip().split()] + vals = [mapper(el) for el in val.text.strip().split('\n')] except: pass else: attrs[attrname] = attr(np.array(vals, dtype=dtype)) - for attrname, attr, in ( ('bond', Bonds), ('angle', Angles), @@ -142,10 +141,11 @@ def parse(self, **kwargs): vals = [] attrs[attrname] = attr(vals) - if not 'masses' in attrs: - attrs['masses'] = Masses(np.zeros(natoms)) - if not 'charges' in attrs: - attrs['charges'] = Charges(np.zeros(natoms, dtype=np.float32)) + if not 'mass' in attrs: + print('Ive got no masses!') + attrs['mass'] = Masses(np.zeros(natoms)) + if not 'charge' in attrs: + attrs['charge'] = Charges(np.zeros(natoms, dtype=np.float32)) attrs = list(attrs.values()) From 8efeca3509475eb885a52ea3b2c68998801b6514 Mon Sep 17 00:00:00 2001 From: CalCraven Date: Mon, 10 Aug 2020 13:14:17 -0500 Subject: [PATCH 2/8] remove extra lines of code, add unit tests, and add information to AUTHORS and CHANGELOG --- package/AUTHORS | 1 + package/CHANGELOG | 3 ++- package/MDAnalysis/topology/HoomdXMLParser.py | 3 +-- testsuite/MDAnalysisTests/topology/test_hoomdxml.py | 7 +++++++ 4 files changed, 11 insertions(+), 3 deletions(-) diff --git a/package/AUTHORS b/package/AUTHORS index 0d729be658a..7defbbae406 100644 --- a/package/AUTHORS +++ b/package/AUTHORS @@ -147,6 +147,7 @@ Chronological list of authors - Andrea Rizzi - William Glass - Marcello Sega + - Cal Craven External code ------------- diff --git a/package/CHANGELOG b/package/CHANGELOG index 6440e9cef5a..d5fe3ded182 100644 --- a/package/CHANGELOG +++ b/package/CHANGELOG @@ -14,11 +14,12 @@ The rules for this file: ------------------------------------------------------------------------------ ??/??/?? tylerjereddy, richardjgowers, IAlibay, hmacdope, orbeckst, cbouy, - lilyminium, daveminh, jbarnoud, yuxuanzhuang, VOD555, ianmkenney + lilyminium, daveminh, jbarnoud, yuxuanzhuang, VOD555, ianmkenney, CalCraven * 2.0.0 Fixes + * Fixed reading in masses and charges from a hoomdxml file (PR #2889) * Bond attribute is no longer set if PDB file contains no CONECT records (Issue #2832) * ResidueAttrs now have Atom as a target class by default; ICodes and diff --git a/package/MDAnalysis/topology/HoomdXMLParser.py b/package/MDAnalysis/topology/HoomdXMLParser.py index a0834c3305b..0ebda165d97 100644 --- a/package/MDAnalysis/topology/HoomdXMLParser.py +++ b/package/MDAnalysis/topology/HoomdXMLParser.py @@ -121,7 +121,7 @@ def parse(self, **kwargs): ): try: val = configuration.find(attrname) - vals = [mapper(el) for el in val.text.strip().split('\n')] + vals = [mapper(el) for el in val.text.strip().split()] except: pass else: @@ -142,7 +142,6 @@ def parse(self, **kwargs): attrs[attrname] = attr(vals) if not 'mass' in attrs: - print('Ive got no masses!') attrs['mass'] = Masses(np.zeros(natoms)) if not 'charge' in attrs: attrs['charge'] = Charges(np.zeros(natoms, dtype=np.float32)) diff --git a/testsuite/MDAnalysisTests/topology/test_hoomdxml.py b/testsuite/MDAnalysisTests/topology/test_hoomdxml.py index cd48bef4886..de49a0e26e1 100644 --- a/testsuite/MDAnalysisTests/topology/test_hoomdxml.py +++ b/testsuite/MDAnalysisTests/topology/test_hoomdxml.py @@ -73,3 +73,10 @@ def test_dihedrals_identity(self, top): for b in ((0, 1, 2, 3), (1, 2, 3, 4), (2, 3, 4, 5), (3, 4, 5, 6)): assert (b in vals) or (b[::-1] in vals) assert ((0, 250, 350, 450) not in vals) + + def test_read_masses(self, top): + for val in top.masses.values: + assert val == 1 + assert ((0, -1, 250, 450) not in top.masses.values) + + From 0deedb9902b9e520b281b5840a4e18850b84dd14 Mon Sep 17 00:00:00 2001 From: CalCraven Date: Mon, 10 Aug 2020 13:54:03 -0500 Subject: [PATCH 3/8] update authors --- package/AUTHORS | 1 + 1 file changed, 1 insertion(+) diff --git a/package/AUTHORS b/package/AUTHORS index 69cebb5e184..cbe9a5c6343 100644 --- a/package/AUTHORS +++ b/package/AUTHORS @@ -148,6 +148,7 @@ Chronological list of authors - William Glass - Marcello Sega - Edis Jakupovic + - Nicholas Craven External code ------------- From 58806dd1a34ac64a22f7a12d76e48d66ad98fd7a Mon Sep 17 00:00:00 2001 From: CalCraven Date: Tue, 11 Aug 2020 10:42:28 -0500 Subject: [PATCH 4/8] Follow format of CHANGELOG and add test_read_charges --- package/CHANGELOG | 6 ++++-- testsuite/MDAnalysisTests/topology/test_hoomdxml.py | 9 ++++++--- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/package/CHANGELOG b/package/CHANGELOG index 7aa10a10d1e..dcf91174cc3 100644 --- a/package/CHANGELOG +++ b/package/CHANGELOG @@ -14,12 +14,14 @@ The rules for this file: ------------------------------------------------------------------------------ ??/??/?? tylerjereddy, richardjgowers, IAlibay, hmacdope, orbeckst, cbouy, - lilyminium, daveminh, jbarnoud, yuxuanzhuang, VOD555, ianmkenney, CalCraven + lilyminium, daveminh, jbarnoud, yuxuanzhuang, VOD555, ianmkenney, + calcraven * 2.0.0 Fixes - * Fixed reading in masses and charges from a hoomdxml file (PR #2889) + * Fixed reading in masses and charges from a hoomdxml file + (Issue #2888, PR #2889) * Bond attribute is no longer set if PDB file contains no CONECT records (Issue #2832) * ResidueAttrs now have Atom as a target class by default; ICodes and diff --git a/testsuite/MDAnalysisTests/topology/test_hoomdxml.py b/testsuite/MDAnalysisTests/topology/test_hoomdxml.py index de49a0e26e1..eae0b9d6b90 100644 --- a/testsuite/MDAnalysisTests/topology/test_hoomdxml.py +++ b/testsuite/MDAnalysisTests/topology/test_hoomdxml.py @@ -75,8 +75,11 @@ def test_dihedrals_identity(self, top): assert ((0, 250, 350, 450) not in vals) def test_read_masses(self, top): - for val in top.masses.values: - assert val == 1 - assert ((0, -1, 250, 450) not in top.masses.values) + assert_almost_equal(top.masses.values, 1.0) + assert ((0, -1, 250, 450) not in top.masses.values) + + def test_read_charges(self, top): + assert_almost_equal(top.charges.values, 0.0) + assert ((1, -1, 0.5, -0.5) not in top.charges.values) From 5c0eb207d2141bb4166112fe3af4fe61d0fad2fc Mon Sep 17 00:00:00 2001 From: Rocco Meli Date: Wed, 12 Aug 2020 02:57:03 +0200 Subject: [PATCH 5/8] Fix rotateby documentation (#2903) * Fixes #2901 * Changes made in this Pull Request: - Added call to rotateby (instead of rotate) - Removed () from atom group --- package/MDAnalysis/transformations/rotate.py | 40 +++++++++++--------- 1 file changed, 22 insertions(+), 18 deletions(-) diff --git a/package/MDAnalysis/transformations/rotate.py b/package/MDAnalysis/transformations/rotate.py index 4e27f81012d..57871292ff3 100644 --- a/package/MDAnalysis/transformations/rotate.py +++ b/package/MDAnalysis/transformations/rotate.py @@ -40,35 +40,39 @@ def rotateby(angle, direction, point=None, ag=None, weights=None, wrap=False): ''' - Rotates the trajectory by a given angle on a given axis. The axis is defined by + Rotates the trajectory by a given angle on a given axis. The axis is defined by the user, combining the direction vector and a point. This point can be the center - of geometry or the center of mass of a user defined AtomGroup, or an array defining + of geometry or the center of mass of a user defined AtomGroup, or an array defining custom coordinates. - + Examples -------- - - e.g. rotate the coordinates by 90 degrees on a axis formed by the [0,0,1] vector and + + e.g. rotate the coordinates by 90 degrees on a axis formed by the [0,0,1] vector and the center of geometry of a given AtomGroup: - + .. code-block:: python - + + from MDAnalysis import transformations + ts = u.trajectory.ts angle = 90 - ag = u.atoms() + ag = u.atoms d = [0,0,1] - rotated = MDAnalysis.transformations.rotate(angle, direction=d, ag=ag)(ts) - + rotated = transformations.rotate.rotateby(angle, direction=d, ag=ag)(ts) + e.g. rotate the coordinates by a custom axis: - + .. code-block:: python + from MDAnalysis import transformations + ts = u.trajectory.ts angle = 90 p = [1,2,3] d = [0,0,1] - rotated = MDAnalysis.transformations.rotate(angle, direction=d, point=point)(ts) - + rotated = transformations.rotate.rotateby(angle, direction=d, point=p)(ts) + Parameters ---------- angle: float @@ -97,7 +101,7 @@ def rotateby(angle, direction, point=None, ag=None, weights=None, wrap=False): Returns ------- MDAnalysis.coordinates.base.Timestep - + Warning ------- Wrapping/unwrapping the trajectory or performing PBC corrections may not be possible @@ -132,7 +136,7 @@ def rotateby(angle, direction, point=None, ag=None, weights=None, wrap=False): center_method = partial(atoms.center, weights, pbc=wrap) else: raise ValueError('A point or an AtomGroup must be specified') - + def wrapped(ts): if point is None: position = center_method() @@ -143,8 +147,8 @@ def wrapped(ts): translation = matrix[:3, 3] ts.positions= np.dot(ts.positions, rotation) ts.positions += translation - + return ts - + return wrapped - + From a9e749c1b29bb3c278153692585f2b6a90b6a923 Mon Sep 17 00:00:00 2001 From: edisj <59893899+edisj@users.noreply.github.com> Date: Wed, 12 Aug 2020 00:44:58 -0700 Subject: [PATCH 6/8] Make H5MD serialize with pickle (#2894) * fix #2890 * added H5PYPicklable class (works in serial but not with driver="mpio" and MPI comm) * mock h5py so that the docs build even in the absence of h5py * tests (anything related to mpi is excluded from coverage) * update CHANGELOG --- package/CHANGELOG | 3 +- package/MDAnalysis/coordinates/H5MD.py | 139 +++++++++++++++--- .../coordinates/pickle_readers.rst | 13 +- .../MDAnalysisTests/utils/test_pickleio.py | 24 ++- 4 files changed, 151 insertions(+), 28 deletions(-) diff --git a/package/CHANGELOG b/package/CHANGELOG index d4c412247e0..ce83b67aed8 100644 --- a/package/CHANGELOG +++ b/package/CHANGELOG @@ -40,7 +40,8 @@ Fixes was thrown when finding D-H pairs via the topology if `hydrogens` was an empty AtomGroup (Issue #2848) * Fixed the DMSParser, allowing the creation of multiple segids sharing - residues with identical resids (Issue #1387, PR #2872) + residues with identical resids (Issue #1387, PR #2872) + * H5MD files are now pickleable with H5PYPicklable (Issue #2890, PR #2894) Enhancements * Refactored analysis.helanal into analysis.helix_analysis diff --git a/package/MDAnalysis/coordinates/H5MD.py b/package/MDAnalysis/coordinates/H5MD.py index bf33eec169b..a49594282d5 100644 --- a/package/MDAnalysis/coordinates/H5MD.py +++ b/package/MDAnalysis/coordinates/H5MD.py @@ -31,7 +31,7 @@ interface of the HDF5 library to improve parallel reads and writes. The HDF5 library and `h5py`_ must be installed; otherwise, H5MD files -cannot be read by MDAnalysis. If `h5py`_ is not installed, a +cannot be read by MDAnalysis. If `h5py`_ is not installed, a :exc:`RuntimeError` is raised. Units @@ -94,10 +94,10 @@ Building parallel h5py and HDF5 on Linux ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -Building a working parallel HDF5/h5py/mpi4py environment can be +Building a working parallel HDF5/h5py/mpi4py environment can be challenging and is often specific to your local computing resources, e.g., the supercomputer that you're running on typically already has -its preferred MPI installation. As a starting point we provide +its preferred MPI installation. As a starting point we provide instructions that worked in a specific, fairly generic environment. These instructions successfully built parallel HDF5/h5py @@ -133,8 +133,8 @@ CC=mpicc HDF5_DIR=$HDF5_PATH python setup.py build python setup.py install -If you have questions or want to share how you managed to build -parallel hdf5/h5py/mpi4py please let everyone know on the +If you have questions or want to share how you managed to build +parallel hdf5/h5py/mpi4py please let everyone know on the `MDAnalysis forums`_. .. _`H5MD`: https://nongnu.org/h5md/index.html @@ -180,6 +180,9 @@ .. automethod:: H5MDReader._reopen +.. autoclass:: H5PYPicklable + :members: + """ import numpy as np @@ -190,6 +193,15 @@ import h5py except ImportError: HAS_H5PY = False + + # Allow building documentation even if h5py is not installed + import types + + class MockH5pyFile: + pass + h5py = types.ModuleType("h5py") + h5py.File = MockH5pyFile + else: HAS_H5PY = True @@ -228,23 +240,23 @@ class H5MDReader(base.ReaderBase): See `h5md documentation `_ for a detailed overview of the H5MD file format. - - The reader attempts to convert units in the trajectory file to - the standard MDAnalysis units (:mod:`MDAnalysis.units`) if + + The reader attempts to convert units in the trajectory file to + the standard MDAnalysis units (:mod:`MDAnalysis.units`) if `convert_units` is set to ``True``. - + Additional data in the *observables* group of the H5MD file are loaded into the :attr:`Timestep.data` dictionary. - + Only 3D-periodic boxes or no periodicity are supported; for no periodicity, :attr:`Timestep.dimensions` will return ``None``. - + Although H5MD can store varying numbers of particles per time step as produced by, e.g., GCMC simulations, MDAnalysis can currently only process a fixed number of particles per step. If the number of particles changes a :exc:`ValueError` is raised. - The :class:`H5MDReader` reads .h5md files with the following + The :class:`H5MDReader` reads .h5md files with the following HDF5 hierarchy: .. code-block:: text @@ -277,21 +289,21 @@ class H5MDReader(base.ReaderBase): \-- (position) \-- [step] , gives frame \-- [time] , gives time - +-- units + +-- unit \-- [value] , gives numpy arrary of positions with shape (n_atoms, 3) +-- unit \-- (velocity) \-- [step] , gives frame \-- [time] , gives time - +-- units + +-- unit \-- [value] , gives numpy arrary of velocities with shape (n_atoms, 3) +-- unit \-- (force) \-- [step] , gives frame \-- [time] , gives time - +-- units + +-- unit \-- [value] , gives numpy arrary of forces with shape (n_atoms, 3) +-- unit @@ -557,13 +569,15 @@ def open_trajectory(self): if self._comm is not None: # can only pass comm argument to h5py.File if driver='mpio' assert self._driver == 'mpio' - self._file = h5py.File(self.filename, 'r', # pragma: no cover - driver=self._driver, - comm=self._comm) + self._file = H5PYPicklable(name=self.filename, # pragma: no cover + mode='r', + driver=self._driver, + comm=self._comm) elif self._driver is not None: - self._file = h5py.File(self.filename, 'r', driver=self._driver) + self._file = H5PYPicklable(name=self.filename, mode='r', + driver=self._driver) else: - self._file = h5py.File(self.filename, 'r') + self._file = H5PYPicklable(name=self.filename, mode='r') # pulls first key out of 'particles' # allows for arbitrary name of group1 in 'particles' self._particle_group = self._file['particles'][ @@ -727,3 +741,88 @@ def has_forces(self): @has_forces.setter def has_forces(self, value: bool): self._has['force'] = value + + def __getstate__(self): + state = self.__dict__.copy() + del state['_particle_group'] + return state + + def __setstate__(self, state): + self.__dict__ = state + self._particle_group = self._file['particles'][ + list(self._file['particles'])[0]] + self[self.ts.frame] + + +class H5PYPicklable(h5py.File): + """H5PY file object (read-only) that can be pickled. + + This class provides a file-like object (as returned by + :class:`h5py.File`) that, + unlike standard Python file objects, + can be pickled. Only read mode is supported. + + When the file is pickled, filename, mode, driver, and comm of + :class:`h5py.File` in the file are saved. On unpickling, the file + is opened by filename, mode, driver. This means that for a successful + unpickle, the original file still has to be accessible with its filename. + + Parameters + ---------- + filename : str or file-like + a filename given a text or byte string. + driver : str (optional) + H5PY file driver used to open H5MD file + + Example + ------- + :: + + f = H5PYPicklable('filename', 'r') + print(f['particles/trajectory/position/value'][0]) + f.close() + + can also be used as context manager:: + + with H5PYPicklable('filename', 'r'): + print(f['particles/trajectory/position/value'][0]) + + Note + ---- + Pickling of an `h5py.File` opened with `driver="mpio"` and an MPI + communicator is currently not supported + + See Also + --------- + :class:`MDAnalysis.lib.picklable_file_io.FileIOPicklable` + :class:`MDAnalysis.lib.picklable_file_io.BufferIOPicklable` + :class:`MDAnalysis.lib.picklable_file_io.TextIOPicklable` + :class:`MDAnalysis.lib.picklable_file_io.GzipPicklable` + :class:`MDAnalysis.lib.picklable_file_io.BZ2Picklable` + + + .. versionadded:: 2.0.0 + """ + + def __getstate__(self): + driver = self.driver + # Current issues: Need a way to retrieve MPI communicator object + # from self and pickle MPI.Comm object. Parallel driver is excluded + # from test because h5py calls for an MPI configuration when driver is + # 'mpio', so this will need to be patched in the test function. + if driver == 'mpio': # pragma: no cover + raise TypeError("Parallel pickling of `h5py.File` with" # pragma: no cover + " 'mpio' driver is currently not supported.") + + return {'name': self.filename, + 'mode': self.mode, + 'driver': driver} + + def __setstate__(self, state): + self.__init__(name=state['name'], + mode=state['mode'], + driver=state['driver']) + + def __getnewargs__(self): + """Override the h5py getnewargs to skip its error message""" + return () diff --git a/package/doc/sphinx/source/documentation_pages/coordinates/pickle_readers.rst b/package/doc/sphinx/source/documentation_pages/coordinates/pickle_readers.rst index 0866590297f..fbcc881ab0c 100644 --- a/package/doc/sphinx/source/documentation_pages/coordinates/pickle_readers.rst +++ b/package/doc/sphinx/source/documentation_pages/coordinates/pickle_readers.rst @@ -1,4 +1,4 @@ -.. Contains the formatted docstrings for the serialization of universe located +.. Contains the formatted docstrings for the serialization of universe located .. mainly in 'MDAnalysis/libs/pickle_file_io.py' .. _serialization: @@ -12,7 +12,7 @@ and what developers should do to serialize a new reader. To make sure every Trajectory reader can be successfully serialized, we implement picklable I/O classes (see :ref:`implemented-fileio`). -When the file is pickled, filename and other necessary attributes of the open +When the file is pickled, filename and other necessary attributes of the open file handle are saved. On unpickling, the file is opened by filename. This means that for a successful unpickle, the original file still has to be accessible with its filename. To retain the current frame of the trajectory, @@ -25,12 +25,12 @@ How to serialize a new reader File Access ^^^^^^^^^^^ -If the new reader uses :func:`util.anyopen()` +If the new reader uses :func:`util.anyopen()` (e.g. :class:`MDAnalysis.coordinates.PDB.PDBReader`), the reading handler can be pickled without modification. If the new reader uses I/O classes from other package (e.g. :class:`MDAnalysis.coordinates.GSD.GSDReader`), -and cannot be pickled natively, create a new picklable class inherited from +and cannot be pickled natively, create a new picklable class inherited from the file class in that package (e.g. :class:`MDAnalysis.coordinates.GSD.GSDPicklable`), adding :func:`__getstate__`, @@ -40,10 +40,10 @@ to allow file handler serialization. To seek or not to seek ^^^^^^^^^^^^^^^^^^^^^^ -Some I/O classes support :func:`seek` and :func:`tell` functions to allow the file +Some I/O classes support :func:`seek` and :func:`tell` functions to allow the file to be pickled with an offset. It is normally not needed for MDAnalysis with random access. But if error occurs during testing, find a way to make the offset work. -Maybe this I/O class supports frame indexing? Maybe the file handler inside this I/O +Maybe this I/O class supports frame indexing? Maybe the file handler inside this I/O class supports offset? For example, in :class:`MDAnalysis.coordinates.TRZ.TRZReader`, @@ -99,3 +99,4 @@ Currently implemented picklable IO Formats * :class:`MDAnalysis.coordinates.GSD.GSDPicklable` * :class:`MDAnalysis.coordinates.TRJ.NCDFPicklable` * :class:`MDAnalysis.coordinates.chemfiles.ChemfilesPicklable` +* :class:`MDAnalysis.coordinates.H5MD.H5PYPicklable` diff --git a/testsuite/MDAnalysisTests/utils/test_pickleio.py b/testsuite/MDAnalysisTests/utils/test_pickleio.py index b005f478c12..bbc7a5a5f23 100644 --- a/testsuite/MDAnalysisTests/utils/test_pickleio.py +++ b/testsuite/MDAnalysisTests/utils/test_pickleio.py @@ -50,6 +50,10 @@ from MDAnalysis.coordinates.chemfiles import ( ChemfilesPicklable ) +from MDAnalysis.coordinates.H5MD import HAS_H5PY +from MDAnalysis.coordinates.H5MD import ( + H5PYPicklable +) from MDAnalysis.tests.datafiles import ( PDB, @@ -58,7 +62,9 @@ MMTF_gz, GMS_ASYMOPT, GSD, - NCDF + NCDF, + TPR_xvf, + H5MD_xvf ) @@ -200,3 +206,19 @@ def test_Chemfiles_with_write_mode(tmpdir): with pytest.raises(ValueError, match=r"Only read mode"): chemfiles_io = ChemfilesPicklable(tmpdir.mkdir("xyz").join('t.xyz'), mode='w') + + +@pytest.mark.skipif(not HAS_H5PY, reason="h5py not installed") +def test_H5MD_pickle(): + h5md_io = H5PYPicklable(H5MD_xvf, 'r') + h5md_io_pickled = pickle.loads(pickle.dumps(h5md_io)) + assert_equal(h5md_io['particles/trajectory/position/value'][0], + h5md_io_pickled['particles/trajectory/position/value'][0]) + + +@pytest.mark.skipif(not HAS_H5PY, reason="h5py not installed") +def test_H5MD_pickle_with_driver(): + h5md_io = H5PYPicklable(H5MD_xvf, 'r', driver='core') + h5md_io_pickled = pickle.loads(pickle.dumps(h5md_io)) + assert_equal(h5md_io['particles/trajectory/position/value'][0], + h5md_io_pickled['particles/trajectory/position/value'][0]) From 6ee8018aa3e2a4f53650c1677539e8d5548a0bb9 Mon Sep 17 00:00:00 2001 From: Oliver Beckstein Date: Wed, 12 Aug 2020 09:15:35 -0700 Subject: [PATCH 7/8] fixed hoomdxml test - import assert_almost_equal - removed asserts that were not needed (and used deprecated constructs) --- testsuite/MDAnalysisTests/topology/test_hoomdxml.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/testsuite/MDAnalysisTests/topology/test_hoomdxml.py b/testsuite/MDAnalysisTests/topology/test_hoomdxml.py index eae0b9d6b90..faa6b949861 100644 --- a/testsuite/MDAnalysisTests/topology/test_hoomdxml.py +++ b/testsuite/MDAnalysisTests/topology/test_hoomdxml.py @@ -20,6 +20,8 @@ # MDAnalysis: A Toolkit for the Analysis of Molecular Dynamics Simulations. # J. Comput. Chem. 32 (2011), 2319--2327, doi:10.1002/jcc.21787 # +from numpy.testing import assert_almost_equal + import MDAnalysis as mda from MDAnalysisTests.topology.base import ParserBase @@ -52,7 +54,7 @@ def test_angles(self, top): def test_dihedrals(self, top): assert len(top.dihedrals.values) == 576 assert isinstance(top.dihedrals.values[0], tuple) - + def test_impropers(self, top): assert len(top.impropers.values) == 0 @@ -76,10 +78,11 @@ def test_dihedrals_identity(self, top): def test_read_masses(self, top): assert_almost_equal(top.masses.values, 1.0) - assert ((0, -1, 250, 450) not in top.masses.values) def test_read_charges(self, top): + # note: the example topology file contains 0 for all charges which + # is the same as the default so this test does not fully test + # reading of charges from the file (#2888) assert_almost_equal(top.charges.values, 0.0) - assert ((1, -1, 0.5, -0.5) not in top.charges.values) - + From 17a07f4302dafb410585fb49aadbdc171059b834 Mon Sep 17 00:00:00 2001 From: Oliver Beckstein Date: Wed, 12 Aug 2020 09:24:10 -0700 Subject: [PATCH 8/8] pep8 fixes --- package/MDAnalysis/topology/HoomdXMLParser.py | 4 ++-- testsuite/MDAnalysisTests/topology/test_hoomdxml.py | 2 -- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/package/MDAnalysis/topology/HoomdXMLParser.py b/package/MDAnalysis/topology/HoomdXMLParser.py index 0ebda165d97..b8e7baa0613 100644 --- a/package/MDAnalysis/topology/HoomdXMLParser.py +++ b/package/MDAnalysis/topology/HoomdXMLParser.py @@ -141,9 +141,9 @@ def parse(self, **kwargs): vals = [] attrs[attrname] = attr(vals) - if not 'mass' in attrs: + if 'mass' not in attrs: attrs['mass'] = Masses(np.zeros(natoms)) - if not 'charge' in attrs: + if 'charge' not in attrs: attrs['charge'] = Charges(np.zeros(natoms, dtype=np.float32)) attrs = list(attrs.values()) diff --git a/testsuite/MDAnalysisTests/topology/test_hoomdxml.py b/testsuite/MDAnalysisTests/topology/test_hoomdxml.py index faa6b949861..018470647f4 100644 --- a/testsuite/MDAnalysisTests/topology/test_hoomdxml.py +++ b/testsuite/MDAnalysisTests/topology/test_hoomdxml.py @@ -84,5 +84,3 @@ def test_read_charges(self, top): # is the same as the default so this test does not fully test # reading of charges from the file (#2888) assert_almost_equal(top.charges.values, 0.0) - -