From 3f7f5d345eb0f650bdb38d46db1091637061cf0c Mon Sep 17 00:00:00 2001 From: Richard Gowers Date: Mon, 8 Jun 2020 21:20:31 +0100 Subject: [PATCH] WIP Issue 2043 deprecate ts write (#2110) * deprecate Timestep argument to Writers (issue #2043) Co-authored-by: Irfan Alibay Co-authored-by: Lily Wang <31115101+lilyminium@users.noreply.github.com> --- package/CHANGELOG | 3 +- package/MDAnalysis/coordinates/DCD.py | 24 ++++- package/MDAnalysis/coordinates/FHIAIMS.py | 9 +- package/MDAnalysis/coordinates/GRO.py | 10 +- package/MDAnalysis/coordinates/MOL2.py | 11 +- package/MDAnalysis/coordinates/NAMDBIN.py | 18 +++- package/MDAnalysis/coordinates/PDB.py | 12 ++- package/MDAnalysis/coordinates/TRJ.py | 44 ++++---- package/MDAnalysis/coordinates/TRR.py | 26 ++++- package/MDAnalysis/coordinates/TRZ.py | 26 ++++- package/MDAnalysis/coordinates/XDR.py | 2 +- package/MDAnalysis/coordinates/XTC.py | 28 ++++- package/MDAnalysis/coordinates/XYZ.py | 22 +++- package/MDAnalysis/coordinates/__init__.py | 10 +- package/MDAnalysis/coordinates/base.py | 51 ++++++--- package/MDAnalysis/coordinates/chemfiles.py | 16 +-- package/MDAnalysis/coordinates/null.py | 2 +- .../coordinates/test_chemfiles.py | 19 ++-- .../MDAnalysisTests/coordinates/test_dcd.py | 2 +- .../coordinates/test_lammps.py | 14 +++ .../coordinates/test_netcdf.py | 14 +-- .../MDAnalysisTests/coordinates/test_trz.py | 2 +- .../coordinates/test_writer_api.py | 102 ++++++++++++++++++ .../MDAnalysisTests/coordinates/test_xdr.py | 10 +- 24 files changed, 351 insertions(+), 126 deletions(-) create mode 100644 testsuite/MDAnalysisTests/coordinates/test_writer_api.py diff --git a/package/CHANGELOG b/package/CHANGELOG index da37b8957d5..a7f2b212c03 100644 --- a/package/CHANGELOG +++ b/package/CHANGELOG @@ -215,7 +215,8 @@ Deprecations * analysis.density.density_from_Universe() (remove in 2.0) * analysis.density.notwithin_coordinates_factory() (remove in 2.0) * analysis.density.density_from_PDB and BfactorDensityCreator (remove in 2.0) - + * Writer.write_next_timestep is deprecated, use write() instead (remove in 2.0) + * Writer.write(Timestep) is deprecated, use either a Universe or AtomGroup 09/05/19 IAlibay, richardjgowers diff --git a/package/MDAnalysis/coordinates/DCD.py b/package/MDAnalysis/coordinates/DCD.py index e6216a266b9..d16fd404350 100644 --- a/package/MDAnalysis/coordinates/DCD.py +++ b/package/MDAnalysis/coordinates/DCD.py @@ -390,17 +390,35 @@ def __init__(self, is_periodic=1, istart=istart) - def write_next_timestep(self, ts): - """Write timestep object into trajectory. + def _write_next_frame(self, ag): + """Write information associated with ``obj`` at current frame into trajectory Parameters ---------- - ts: TimeStep + ag : AtomGroup or Universe See Also -------- :meth:`DCDWriter.write` takes a more general input + + + .. deprecated:: 1.0.0 + Deprecated use of Timestep as argument. To be removed in version 2.0 + .. versionchanged:: 1.0.0 + Added ability to pass AtomGroup or Universe. + Renamed from `write_next_timestep` to `_write_next_frame`. """ + if isinstance(ag, base.Timestep): + ts = ag + else: + try: + ts = ag.ts + except AttributeError: + try: + # Universe? + ts = ag.trajectory.ts + except AttributeError: + raise TypeError("No Timestep found in ag argument") xyz = ts.positions.copy() dimensions = ts.dimensions.copy() diff --git a/package/MDAnalysis/coordinates/FHIAIMS.py b/package/MDAnalysis/coordinates/FHIAIMS.py index 11fb1ed03ce..f475b2c7620 100644 --- a/package/MDAnalysis/coordinates/FHIAIMS.py +++ b/package/MDAnalysis/coordinates/FHIAIMS.py @@ -267,22 +267,19 @@ def __init__(self, filename, convert_units=True, n_atoms=None, **kwargs): self.filename = util.filename(filename, ext='.in', keep=True) self.n_atoms = n_atoms - def write(self, obj): + def _write_next_frame(self, obj): """Write selection at current trajectory frame to file. Parameters ----------- - obj : AtomGroup or Universe or :class:`Timestep` - + obj : AtomGroup or Universe """ # write() method that complies with the Trajectory API - + # TODO 2.0: Remove timestep logic try: - # make sure to use atoms (Issue 46) ag_or_ts = obj.atoms # can write from selection == Universe (Issue 49) - except AttributeError: if isinstance(obj, base.Timestep): ag_or_ts = obj.copy() diff --git a/package/MDAnalysis/coordinates/GRO.py b/package/MDAnalysis/coordinates/GRO.py index 981ea1f7c00..3e091816860 100644 --- a/package/MDAnalysis/coordinates/GRO.py +++ b/package/MDAnalysis/coordinates/GRO.py @@ -344,7 +344,7 @@ def write(self, obj): Parameters ----------- - obj : AtomGroup or Universe or :class:`Timestep` + obj : AtomGroup or Universe Note ---- @@ -357,6 +357,9 @@ def write(self, obj): *resName* and *atomName* are truncated to a maximum of 5 characters .. versionchanged:: 0.16.0 `frame` kwarg has been removed + .. deprecated:: 1.0.0 + Deprecated calling with Timestep, use AtomGroup or Universe. + To be removed in version 2.0. """ # write() method that complies with the Trajectory API @@ -368,6 +371,11 @@ def write(self, obj): except AttributeError: if isinstance(obj, base.Timestep): + warnings.warn( + 'Passing a Timestep to write is deprecated, ' + 'and will be removed in 2.0; ' + 'use either an AtomGroup or Universe', + DeprecationWarning) ag_or_ts = obj.copy() else: raise_from(TypeError("No Timestep found in obj argument"), None) diff --git a/package/MDAnalysis/coordinates/MOL2.py b/package/MDAnalysis/coordinates/MOL2.py index 696128913ef..9b4d0b4662a 100644 --- a/package/MDAnalysis/coordinates/MOL2.py +++ b/package/MDAnalysis/coordinates/MOL2.py @@ -375,21 +375,16 @@ def encode_block(self, obj): molecule[1] = molecule_1_store return return_val - def write(self, obj): + def _write_next_frame(self, obj): """Write a new frame to the MOL2 file. Parameters ---------- obj : AtomGroup or Universe - """ - self.write_next_timestep(obj) - def write_next_timestep(self, obj): - """Write a new frame to the MOL2 file. - Parameters - ---------- - obj : AtomGroup or Universe + .. versionchanged:: 1.0.0 + Renamed from `write_next_timestep` to `_write_next_frame`. """ block = self.encode_block(obj) self.file.writelines(block) diff --git a/package/MDAnalysis/coordinates/NAMDBIN.py b/package/MDAnalysis/coordinates/NAMDBIN.py index ef4a5bb68ea..7b70c6e91c2 100755 --- a/package/MDAnalysis/coordinates/NAMDBIN.py +++ b/package/MDAnalysis/coordinates/NAMDBIN.py @@ -113,15 +113,23 @@ def __init__(self, filename, n_atoms=None, **kwargs): """ self.filename = util.filename(filename) - def write(self, obj): - """Write obj at current trajectory frame to file. + def _write_next_frame(self, obj): + """Write information associated with ``obj`` at current frame into trajectory + Parameters ---------- - obj : :class:`~MDAnalysis.core.groups.AtomGroup` or :class:`~MDAnalysis.core.universe.Universe` or a :class:`Timestep` - write coordinate information associate with `obj` - """ + obj : :class:`~MDAnalysis.core.groups.AtomGroup` or :class:`~MDAnalysis.core.universe.Universe` + write coordinate information associated with `obj` + + + .. versionchanged:: 1.0.0 + Renamed from `write` to `_write_next_frame`. + .. deprecated:: 1.0.0 + Passing a Timestep is deprecated for removal in version 2.0 + """ + # TODO 2.0: Remove Timestep logic if isinstance(obj, base.Timestep): n_atoms = obj.n_atoms coor = obj.positions.reshape(n_atoms*3) diff --git a/package/MDAnalysis/coordinates/PDB.py b/package/MDAnalysis/coordinates/PDB.py index 23f8914c79b..649bde13808 100644 --- a/package/MDAnalysis/coordinates/PDB.py +++ b/package/MDAnalysis/coordinates/PDB.py @@ -825,7 +825,7 @@ def _update_frame(self, obj): * :attr:`PDBWriter.timestep` (the underlying trajectory :class:`~MDAnalysis.coordinates.base.Timestep`) - Before calling :meth:`write_next_timestep` this method **must** be + Before calling :meth:`_write_next_frame` this method **must** be called at least once to enable extracting topology information from the current frame. """ @@ -864,7 +864,7 @@ def write(self, obj): # Issue 105: with write() ONLY write a single frame; use # write_all_timesteps() to dump everything in one go, or do the # traditional loop over frames - self.write_next_timestep(self.ts, multiframe=self._multiframe) + self._write_next_frame(self.ts, multiframe=self._multiframe) self._write_pdb_bonds() # END record is written when file is being close()d @@ -907,7 +907,7 @@ def write_all_timesteps(self, obj): for framenumber in range(start, len(traj), step): traj[framenumber] - self.write_next_timestep(self.ts, multiframe=True) + self._write_next_frame(self.ts, multiframe=True) self._write_pdb_bonds() self.close() @@ -915,7 +915,7 @@ def write_all_timesteps(self, obj): # Set the trajectory to the starting position traj[start] - def write_next_timestep(self, ts=None, **kwargs): + def _write_next_frame(self, ts=None, **kwargs): '''write a new timestep to the PDB file :Keywords: @@ -931,6 +931,10 @@ def write_next_timestep(self, ts=None, **kwargs): argument, :meth:`PDBWriter._update_frame` *must* be called with the :class:`~MDAnalysis.core.groups.AtomGroup.Universe` as its argument so that topology information can be gathered. + + + .. versionchanged:: 1.0.0 + Renamed from `write_next_timestep` to `_write_next_frame`. ''' if ts is None: try: diff --git a/package/MDAnalysis/coordinates/TRJ.py b/package/MDAnalysis/coordinates/TRJ.py index e35d606e5ad..d87a4ca1824 100644 --- a/package/MDAnalysis/coordinates/TRJ.py +++ b/package/MDAnalysis/coordinates/TRJ.py @@ -869,7 +869,6 @@ def __init__(self, self.dt = dt self.remarks = remarks or "AMBER NetCDF format (MDAnalysis.coordinates.trj.NCDFWriter)" - self.ts = None # when/why would this be assigned?? self._first_frame = True # signals to open trajectory self.trjfile = None # open on first write with _init_netcdf() self.periodic = None # detect on first write @@ -972,39 +971,48 @@ def _init_netcdf(self, periodic=True): self._first_frame = False self.trjfile = ncfile - def is_periodic(self, ts=None): - """Test if `Timestep` contains a periodic trajectory. + def is_periodic(self, ts): + """Test if timestep ``ts`` contains a periodic box. Parameters ---------- ts : :class:`Timestep` :class:`Timestep` instance containing coordinates to - be written to trajectory file; default is the current - timestep + be written to trajectory file Returns ------- bool Return ``True`` if `ts` contains a valid simulation box """ - ts = ts if ts is not None else self.ts return np.all(ts.dimensions > 0) - def write_next_timestep(self, ts=None): - """write a new timestep to the trj file + def _write_next_frame(self, ag): + """Write information associated with ``ag`` at current frame into trajectory Parameters ---------- - ts : :class:`Timestep` - :class:`Timestep` instance containing coordinates to - be written to trajectory file; default is the current - timestep + ag : AtomGroup or Universe + + + .. deprecated:: 1.0.0 + Deprecated using Timestep. To be removed in version 2.0. + .. versionchanged:: 1.0.0 + Added ability to use either AtomGroup or Universe. + Renamed from `write_next_timestep` to `_write_next_frame`. """ - if ts is None: - ts = self.ts - if ts is None: - raise IOError( - "NCDFWriter: no coordinate data to write to trajectory file") + if isinstance(ag, base.Timestep): + ts = ag + else: + try: + # Atomgroup? + ts = ag.ts + except AttributeError: + try: + # Universe? + ts = ag.trajectory.ts + except AttributeError: + raise TypeError("No Timestep found in ag argument") if ts.n_atoms != self.n_atoms: raise IOError( @@ -1020,7 +1028,7 @@ def _write_next_timestep(self, ts): """Write coordinates and unitcell information to NCDF file. Do not call this method directly; instead use - :meth:`write_next_timestep` because some essential setup is done + :meth:`write` because some essential setup is done there before writing the first frame. Based on Joshua Adelman's `netcdf4storage.py`_ in `Issue 109`_. diff --git a/package/MDAnalysis/coordinates/TRR.py b/package/MDAnalysis/coordinates/TRR.py index e92871f77b0..bd77ffea1f0 100644 --- a/package/MDAnalysis/coordinates/TRR.py +++ b/package/MDAnalysis/coordinates/TRR.py @@ -33,6 +33,7 @@ """ from __future__ import absolute_import +from . import base from .XDR import XDRBaseReader, XDRBaseWriter from ..lib.formats.libmdaxdr import TRRFile from ..lib.mdamath import triclinic_vectors, triclinic_box @@ -58,18 +59,37 @@ class TRRWriter(XDRBaseWriter): 'force': 'kJ/(mol*nm)'} _file = TRRFile - def write_next_timestep(self, ts): - """Write timestep object into trajectory. + def _write_next_frame(self, ag): + """Write information associated with ``ag`` at current frame into trajectory Parameters ---------- - ts : :class:`~base.Timestep` + ag : AtomGroup or Universe See Also -------- .write(AtomGroup/Universe/TimeStep) The normal write() method takes a more general input + + + .. versionchanged:: 1.0.0 + Renamed from `write_next_timestep` to `_write_next_frame`. + .. deprecated:: 1.0.0 + Deprecated the use of Timestep as arguments to write. Use either + an AtomGroup or Universe. To be removed in version 2.0. """ + if isinstance(ag, base.Timestep): + ts = ag + else: + try: + ts = ag.ts + except AttributeError: + try: + # special case: can supply a Universe, too... + ts = ag.trajectory.ts + except AttributeError: + raise TypeError("No Timestep found in ag argument") + xyz = None if ts.has_positions: xyz = ts.positions.copy() diff --git a/package/MDAnalysis/coordinates/TRZ.py b/package/MDAnalysis/coordinates/TRZ.py index 8f2d1d5fff0..6598f077b1a 100644 --- a/package/MDAnalysis/coordinates/TRZ.py +++ b/package/MDAnalysis/coordinates/TRZ.py @@ -530,10 +530,30 @@ def _writeheader(self, title): out['nrec'] = 10 out.tofile(self.trzfile) - def write_next_timestep(self, ts): + def _write_next_frame(self, obj): + """Write information associated with ``obj`` at current frame into trajectory + + Parameters + ---------- + ag : AtomGroup or Universe + + + .. versionchanged:: 1.0.0 + Renamed from `write_next_timestep` to `_write_next_frame`. + """ # Check size of ts is same as initial - if not ts.n_atoms == self.n_atoms: - raise ValueError("Number of atoms in ts different to initialisation") + # TODO: Remove Timestep logic in 2.0 + if isinstance(obj, base.Timestep): + ts = obj + if not ts.n_atoms == self.n_atoms: + raise ValueError("Number of atoms in ts different to initialisation") + else: + try: # atomgroup? + ts = obj.ts + except AttributeError: # universe? + ts = obj.trajectory.ts + if not obj.atoms.n_atoms == self.n_atoms: + raise ValueError("Number of atoms in ts different to initialisation") # Gather data, faking it when unavailable data = {} diff --git a/package/MDAnalysis/coordinates/XDR.py b/package/MDAnalysis/coordinates/XDR.py index bb7bd010433..8095323ade9 100644 --- a/package/MDAnalysis/coordinates/XDR.py +++ b/package/MDAnalysis/coordinates/XDR.py @@ -92,7 +92,7 @@ class XDRBaseReader(base.ReaderBase): """Base class for libmdaxdr file formats xtc and trr This class handles integration of XDR based formats into MDAnalysis. The - XTC and TRR classes only implement `write_next_timestep` and + XTC and TRR classes only implement `_write_next_frame` and `_frame_to_ts`. .. _offsets-label: diff --git a/package/MDAnalysis/coordinates/XTC.py b/package/MDAnalysis/coordinates/XTC.py index a581748ed01..125db00eacd 100644 --- a/package/MDAnalysis/coordinates/XTC.py +++ b/package/MDAnalysis/coordinates/XTC.py @@ -33,6 +33,7 @@ """ from __future__ import absolute_import +from . import base from .XDR import XDRBaseReader, XDRBaseWriter from ..lib.formats.libmdaxdr import XTCFile from ..lib.mdamath import triclinic_vectors, triclinic_box @@ -70,18 +71,37 @@ def __init__(self, filename, n_atoms, convert_units=True, **kwargs) self.precision = precision - def write_next_timestep(self, ts): - """Write timestep object into trajectory. + def _write_next_frame(self, ag): + """Write information associated with ``ag`` at current frame into trajectory Parameters ---------- - ts : :class:`~base.Timestep` + ag : AtomGroup or Universe See Also -------- - .write(AtomGroup/Universe/TimeStep) + .write(AtomGroup/Universe) The normal write() method takes a more general input + + + .. deprecated:: 1.0.0 + Deprecated using Timestep. To be removed in version 2.0. + .. versionchanged:: 1.0.0 + Added ability to use either AtomGroup or Universe. """ + if isinstance(ag, base.Timestep): + ts = ag + else: + try: + # Atomgroup? + ts = ag.ts + except AttributeError: + try: + # Universe? + ts = ag.trajectory.ts + except AttributeError: + raise TypeError("No Timestep found in ag argument") + xyz = ts.positions.copy() time = ts.time step = ts.frame diff --git a/package/MDAnalysis/coordinates/XYZ.py b/package/MDAnalysis/coordinates/XYZ.py index 4eb6af44502..88678e6e64b 100644 --- a/package/MDAnalysis/coordinates/XYZ.py +++ b/package/MDAnalysis/coordinates/XYZ.py @@ -89,6 +89,7 @@ import os import errno import numpy as np +import warnings import logging logger = logging.getLogger('MDAnalysis.coordinates.XYZ') @@ -201,6 +202,11 @@ def write(self, obj): obj : Universe or AtomGroup The :class:`~MDAnalysis.core.groups.AtomGroup` or :class:`~MDAnalysis.core.universe.Universe` to write. + + + .. deprecated:: 1.0.0 + Deprecated the use of Timestep as arguments to write. Use either an + AtomGroup or Universe. To be removed in version 2.0. """ # prepare the Timestep and extract atom names if possible # (The way it is written it should be possible to write @@ -210,6 +216,11 @@ def write(self, obj): atoms = obj.atoms except AttributeError: if isinstance(obj, base.Timestep): + warnings.warn( + 'Passing a Timestep to write is deprecated, ' + 'and will be removed in 2.0; ' + 'use either an AtomGroup or Universe', + DeprecationWarning) ts = obj else: six.raise_from(TypeError("No Timestep found in obj argument"), None) @@ -228,15 +239,16 @@ def write(self, obj): # update atom names self.atomnames = self._get_atoms_elements_or_names(atoms) - self.write_next_timestep(ts) + self._write_next_frame(ts) - def write_next_timestep(self, ts=None): + def _write_next_frame(self, ts=None): """ Write coordinate information in *ts* to the trajectory .. versionchanged:: 1.0.0 Print out :code:`remark` if present, otherwise use generic one (Issue #2692). + Renamed from `write_next_timestep` to `_write_next_frame`. """ if ts is None: if not hasattr(self, 'ts'): @@ -255,9 +267,9 @@ def write_next_timestep(self, ts=None): else: if (not isinstance(self.atomnames, itertools.cycle) and len(self.atomnames) != ts.n_atoms): - logger.info('Trying to write a TimeStep with unkown atoms. ' - 'Expected {}, got {}. Try using "write" if you are ' - 'using "write_next_timestep" directly'.format( + logger.info('Trying to write a TimeStep with unknown atoms. ' + 'Expected {} atoms, got {}. Try using "write" if you are ' + 'using "_write_next_frame" directly'.format( len(self.atomnames), ts.n_atoms)) self.atomnames = np.array([self.atomnames[0]] * ts.n_atoms) diff --git a/package/MDAnalysis/coordinates/__init__.py b/package/MDAnalysis/coordinates/__init__.py index d8548f24648..361489a9ab0 100644 --- a/package/MDAnalysis/coordinates/__init__.py +++ b/package/MDAnalysis/coordinates/__init__.py @@ -622,14 +622,12 @@ class can choose an appropriate reader automatically. Signature:: - W = TrajectoryWriter(filename,n_atoms,**kwargs) - W.write_next_timestep(Timestep) + with TrajectoryWriter(filename, n_atoms, **kwargs) as w: + w.write(Universe) # write a whole universe or:: - W.write(AtomGroup) # write a selection - W.write(Universe) # write a whole universe - W.write(Timestep) # same as write_next_timestep() + w.write(AtomGroup) # write a selection of Atoms from Universe Methods @@ -639,8 +637,6 @@ class can choose an appropriate reader automatically. opens *filename* and writes header if required by format ``write(obj)`` write Timestep data in *obj* - ``write_next_timestep([timestep])`` - write data in *timestep* to trajectory file ``convert_dimensions_to_unitcell(timestep)`` take the dimensions from the timestep and convert to the native unitcell representation of the format diff --git a/package/MDAnalysis/coordinates/base.py b/package/MDAnalysis/coordinates/base.py index e51101cfc7c..640d9af78f9 100644 --- a/package/MDAnalysis/coordinates/base.py +++ b/package/MDAnalysis/coordinates/base.py @@ -2183,6 +2183,11 @@ class WriterBase(six.with_metaclass(_Writermeta, IOBase)): See Trajectory API definition in :mod:`MDAnalysis.coordinates.__init__` for the required attributes and methods. + + + .. deprecated:: 1.0.0 + :func:`write_next_timestep` has been deprecated, please use + :func:`write` instead. """ def convert_dimensions_to_unitcell(self, ts, inplace=True): @@ -2204,26 +2209,46 @@ def write(self, obj): Parameters ---------- - obj : :class:`~MDAnalysis.core.groups.AtomGroup` or :class:`~MDAnalysis.core.universe.Universe` or a :class:`Timestep` + obj : :class:`~MDAnalysis.core.groups.AtomGroup` or :class:`~MDAnalysis.core.universe.Universe` write coordinate information associate with `obj` Note ---- The size of the `obj` must be the same as the number of atoms provided when setting up the trajectory. + + + .. deprecated:: 1.0.0 + Deprecated the use of Timestep as arguments to write. Use either + an AtomGroup or Universe. To be removed in version 2.0. """ if isinstance(obj, Timestep): - ts = obj - else: - try: - ts = obj.ts - except AttributeError: - try: - # special case: can supply a Universe, too... - ts = obj.trajectory.ts - except AttributeError: - six.raise_from(TypeError("No Timestep found in obj argument"), None) - return self.write_next_timestep(ts) + warnings.warn( + 'Passing a Timestep to write is deprecated, ' + 'and will be removed in 2.0; ' + 'use either an AtomGroup or Universe', + DeprecationWarning) + + return self._write_next_frame(obj) + + def write_next_timestep(self, obj): + """Write current timestep, using the supplied ``obj``. + + Parameters + ---------- + obj : :class:`~MDAnalysis.core.groups.AtomGroup` or :class:`~MDAnalysis.core.universe.Universe` + write coordinate information associated with ``obj`` + + + .. deprecated:: 1.0.0 + Deprecated, use write() instead + """ + warnings.warn( + 'Writer.write_next_timestep is deprecated, ' + 'and will be removed in 2.0; ' + 'use Writer.write()', + DeprecationWarning) + return self.write(obj) def __del__(self): self.close() @@ -2257,8 +2282,6 @@ def has_valid_coordinates(self, criteria, x): x = np.ravel(x) return np.all(criteria["min"] < x) and np.all(x <= criteria["max"]) - # def write_next_timestep(self, ts=None) - class SingleFrameReaderBase(ProtoReader): """Base class for Readers that only have one frame. diff --git a/package/MDAnalysis/coordinates/chemfiles.py b/package/MDAnalysis/coordinates/chemfiles.py index c7fbe842c2c..673c62e2826 100644 --- a/package/MDAnalysis/coordinates/chemfiles.py +++ b/package/MDAnalysis/coordinates/chemfiles.py @@ -273,8 +273,8 @@ def close(self): self._file.close() self._closed = True - def write(self, obj): - """Write object `obj` at current trajectory frame to file. + def _write_next_frame(self, obj): + """Write information associated with ``obj`` at current frame into trajectory. Topology for the output is taken from the `obj` or default to the value of the `topology` keyword supplied to the :class:`ChemfilesWriter` @@ -286,10 +286,11 @@ def write(self, obj): Parameters ---------- - obj : Universe or AtomGroup + obj : AtomGroup or Universe The :class:`~MDAnalysis.core.groups.AtomGroup` or :class:`~MDAnalysis.core.universe.Universe` to write. """ + # TODO 2.0: Remove timestep logic if hasattr(obj, "atoms"): if hasattr(obj, 'universe'): # For AtomGroup and children (Residue, ResidueGroup, Segment) @@ -311,16 +312,7 @@ def write(self, obj): frame = self._timestep_to_chemfiles(ts) frame.topology = self._topology_to_chemfiles(obj, len(frame.atoms)) - self._file.write(frame) - - def write_next_timestep(self, ts): - """Write timestep object into trajectory. - Parameters - ---------- - ts: TimeStep - """ - frame = self._timestep_to_chemfiles(ts) self._file.write(frame) def _timestep_to_chemfiles(self, ts): diff --git a/package/MDAnalysis/coordinates/null.py b/package/MDAnalysis/coordinates/null.py index c3a4e5118f8..3ba8cda6087 100644 --- a/package/MDAnalysis/coordinates/null.py +++ b/package/MDAnalysis/coordinates/null.py @@ -56,5 +56,5 @@ class NullWriter(base.WriterBase): def __init__(self, filename, **kwargs): pass - def write_next_timestep(self, ts=None): + def _write_next_frame(self, obj): pass diff --git a/testsuite/MDAnalysisTests/coordinates/test_chemfiles.py b/testsuite/MDAnalysisTests/coordinates/test_chemfiles.py index d21506f3fe5..21dd97759c9 100644 --- a/testsuite/MDAnalysisTests/coordinates/test_chemfiles.py +++ b/testsuite/MDAnalysisTests/coordinates/test_chemfiles.py @@ -162,12 +162,12 @@ def test_write_topology(self, tmpdir): # Manually setting the topology when creating the ChemfilesWriter # (1) from an object with ChemfilesWriter(outfile, topology=u) as writer: - writer.write_next_timestep(u.trajectory.ts) + writer.write(u) self.check_topology(datafiles.CONECT, outfile) # (2) from a file with ChemfilesWriter(outfile, topology=datafiles.CONECT) as writer: - writer.write_next_timestep(u.trajectory.ts) + writer.write(u) # FIXME: this does not work, since chemfiles also insert the bonds # which are implicit in PDB format (between standard residues), while # MDAnalysis only read the explicit CONNECT records. @@ -175,9 +175,11 @@ def test_write_topology(self, tmpdir): # self.check_topology(datafiles.CONECT, outfile) def test_write_velocities(self, tmpdir): - ts = mda.coordinates.base.Timestep(4, velocities=True) - ts.dimensions = [20, 30, 41, 90, 90, 90] + u = mda.Universe.empty(4, trajectory=True) + u.add_TopologyAttr('type', values=['H', 'H', 'H', 'H']) + ts = u.trajectory.ts + ts.dimensions = [20, 30, 41, 90, 90, 90] ts.positions = [ [1, 1, 1], [2, 2, 2], @@ -191,17 +193,10 @@ def test_write_velocities(self, tmpdir): [40, 40, 40], ] - u = mda.Universe.empty(4) - u.add_TopologyAttr('type') - u.atoms[0].type = "H" - u.atoms[1].type = "H" - u.atoms[2].type = "H" - u.atoms[3].type = "H" - outfile = "chemfiles-write-velocities.lmp" with tmpdir.as_cwd(): with ChemfilesWriter(outfile, topology=u, chemfiles_format='LAMMPS Data') as writer: - writer.write_next_timestep(ts) + writer.write(u) with open(outfile) as file: content = file.read() diff --git a/testsuite/MDAnalysisTests/coordinates/test_dcd.py b/testsuite/MDAnalysisTests/coordinates/test_dcd.py index dccfca070b9..e21e6e6e420 100644 --- a/testsuite/MDAnalysisTests/coordinates/test_dcd.py +++ b/testsuite/MDAnalysisTests/coordinates/test_dcd.py @@ -280,7 +280,7 @@ def test_other_writer(universe_dcd, tmpdir, ext, decimal): outfile = str(tmpdir.join("test.{}".format(ext))) with t.OtherWriter(outfile) as W: for ts in universe_dcd.trajectory: - W.write_next_timestep(ts) + W.write(universe_dcd) uw = mda.Universe(PSF, outfile) # check that the coordinates are identical for each time step diff --git a/testsuite/MDAnalysisTests/coordinates/test_lammps.py b/testsuite/MDAnalysisTests/coordinates/test_lammps.py index 1e775b94980..e3c88539ab3 100644 --- a/testsuite/MDAnalysisTests/coordinates/test_lammps.py +++ b/testsuite/MDAnalysisTests/coordinates/test_lammps.py @@ -154,6 +154,20 @@ def test_Writer_numerical_attrs(self, attr, LAMMPSDATAWriter): decimal=6) +def test_datawriter_universe(tmpdir): + fn = str(tmpdir.join('out.data')) + + u = mda.Universe(LAMMPSdata_mini) + + with mda.Writer(fn, n_atoms=len(u.atoms)) as w: + w.write(u) + + u2 = mda.Universe(fn) + + assert_almost_equal(u.atoms.positions, u2.atoms.positions) + assert_almost_equal(u.dimensions, u2.dimensions) + + class TestLAMMPSDATAWriter_data_partial(TestLAMMPSDATAWriter): N_kept = 5 diff --git a/testsuite/MDAnalysisTests/coordinates/test_netcdf.py b/testsuite/MDAnalysisTests/coordinates/test_netcdf.py index de040e9a092..81409b82d00 100644 --- a/testsuite/MDAnalysisTests/coordinates/test_netcdf.py +++ b/testsuite/MDAnalysisTests/coordinates/test_netcdf.py @@ -664,7 +664,7 @@ def test_OtherWriter(self, universe, outfile): def _copy_traj(self, writer, universe): for ts in universe.trajectory: - writer.write_next_timestep(ts) + writer.write(universe) def _check_new_traj(self, universe, outfile): uw = mda.Universe(self.topology, outfile) @@ -724,7 +724,7 @@ def test_TRR2NCDF(self, outfile): with mda.Writer(outfile, trr.trajectory.n_atoms, velocities=True, format="ncdf") as W: for ts in trr.trajectory: - W.write_next_timestep(ts) + W.write(trr) uw = mda.Universe(GRO, outfile) @@ -877,7 +877,7 @@ def test_writer_units(self, outfile, var, expected): with mda.Writer(outfile, trr.trajectory.n_atoms, velocities=True, forces=True, format='ncdf') as W: for ts in trr.trajectory: - W.write_next_timestep(ts) + W.write(trr) with netcdf.netcdf_file(outfile, mode='r') as ncdf: unit = ncdf.variables[var].units.decode('utf-8') @@ -902,11 +902,3 @@ def test_wrong_n_atoms(self, outfile): u = make_Universe(trajectory=True) with pytest.raises(IOError): w.write(u.trajectory.ts) - - def test_no_ts(self, outfile): - # no ts supplied at any point - from MDAnalysis.coordinates.TRJ import NCDFWriter - - with NCDFWriter(outfile, 100) as w: - with pytest.raises(IOError): - w.write_next_timestep() diff --git a/testsuite/MDAnalysisTests/coordinates/test_trz.py b/testsuite/MDAnalysisTests/coordinates/test_trz.py index 5e814a0402e..7f1c542241b 100644 --- a/testsuite/MDAnalysisTests/coordinates/test_trz.py +++ b/testsuite/MDAnalysisTests/coordinates/test_trz.py @@ -153,7 +153,7 @@ def test_write_trajectory(self, universe, outfile): def _copy_traj(self, writer, universe, outfile): for ts in universe.trajectory: - writer.write_next_timestep(ts) + writer.write(universe) writer.close() uw = mda.Universe(TRZ_psf, outfile) diff --git a/testsuite/MDAnalysisTests/coordinates/test_writer_api.py b/testsuite/MDAnalysisTests/coordinates/test_writer_api.py new file mode 100644 index 00000000000..ad578732733 --- /dev/null +++ b/testsuite/MDAnalysisTests/coordinates/test_writer_api.py @@ -0,0 +1,102 @@ +# -*- Mode: python; tab-width: 4; indent-tabs-mode:nil; coding:utf-8 -*- +# vim: tabstop=4 expandtab shiftwidth=4 softtabstop=4 fileencoding=utf-8 +# +# MDAnalysis --- https://www.mdanalysis.org +# Copyright (c) 2006-2017 The MDAnalysis Development Team and contributors +# (see the file AUTHORS for the full list of names) +# +# Released under the GNU Public Licence, v2 or any higher version +# +# Please cite your use of MDAnalysis in published work: +# +# R. J. Gowers, M. Linke, J. Barnoud, T. J. E. Reddy, M. N. Melo, S. L. Seyler, +# D. L. Dotson, J. Domanski, S. Buchoux, I. M. Kenney, and O. Beckstein. +# MDAnalysis: A Python package for the rapid analysis of molecular dynamics +# simulations. In S. Benthall and S. Rostrup editors, Proceedings of the 15th +# Python in Science Conference, pages 102-109, Austin, TX, 2016. SciPy. +# +# N. Michaud-Agrawal, E. J. Denning, T. B. Woolf, and O. Beckstein. +# MDAnalysis: A Toolkit for the Analysis of Molecular Dynamics Simulations. +# J. Comput. Chem. 32 (2011), 2319--2327, doi:10.1002/jcc.21787 +# +from __future__ import absolute_import + +import itertools +import pytest + +import MDAnalysis as mda + + +# grab all known writers +# sort so test order is predictable for parallel tests +writers = sorted(set(mda._MULTIFRAME_WRITERS.values()) | + set(mda._SINGLEFRAME_WRITERS.values()), + key=lambda x: x.__name__) +known_ts_haters = [ + mda.coordinates.MOL2.MOL2Writer, + mda.coordinates.PDB.PDBWriter, + mda.coordinates.PDB.MultiPDBWriter, + mda.coordinates.PQR.PQRWriter, + mda.coordinates.PDBQT.PDBQTWriter, + mda.coordinates.LAMMPS.DATAWriter, + mda.coordinates.CRD.CRDWriter, +] + + +@pytest.mark.parametrize('writer', [w for w in writers + if not w in known_ts_haters]) +def test_ts_deprecated(writer, tmpdir): + u = mda.Universe.empty(10, trajectory=True) + + if writer == mda.coordinates.chemfiles.ChemfilesWriter: + # chemfiles Writer exists but doesn't work without chemfiles + if not mda.coordinates.chemfiles.check_chemfiles_version(): + pytest.skip("Chemfiles not available") + fn = str(tmpdir.join('out.xtc')) + else: + fn = str(tmpdir.join('out.traj')) + + with writer(fn, n_atoms=u.atoms.n_atoms) as w: + with pytest.warns(DeprecationWarning): + w.write(u.trajectory.ts) + + +@pytest.mark.parametrize('writer', writers) +def test_write_with_atomgroup(writer, tmpdir): + u = mda.Universe.empty(10, trajectory=True) + + if writer == mda.coordinates.chemfiles.ChemfilesWriter: + # chemfiles Writer exists but doesn't work without chemfiles + if not mda.coordinates.chemfiles.check_chemfiles_version(): + pytest.skip("Chemfiles not available") + fn = str(tmpdir.join('out.xtc')) + elif writer == mda.coordinates.MOL2.MOL2Writer: + pytest.skip("MOL2 only writes MOL2 back out") + elif writer == mda.coordinates.LAMMPS.DATAWriter: + pytest.skip("DATAWriter requires integer atom types") + else: + fn = str(tmpdir.join('out.traj')) + + with writer(fn, n_atoms=u.atoms.n_atoms) as w: + w.write(u.atoms) + + +@pytest.mark.parametrize('writer', writers) +def test_write_with_universe(writer, tmpdir): + u = mda.Universe.empty(10, trajectory=True) + + if writer == mda.coordinates.chemfiles.ChemfilesWriter: + # chemfiles Writer exists but doesn't work without chemfiles + if not mda.coordinates.chemfiles.check_chemfiles_version(): + pytest.skip("Chemfiles not available") + fn = str(tmpdir.join('out.xtc')) + elif writer == mda.coordinates.MOL2.MOL2Writer: + pytest.skip("MOL2 only writes MOL2 back out") + elif writer == mda.coordinates.LAMMPS.DATAWriter: + pytest.skip("DATAWriter requires integer atom types") + else: + fn = str(tmpdir.join('out.traj')) + + with writer(fn, n_atoms=10) as w: + w.write(u) + diff --git a/testsuite/MDAnalysisTests/coordinates/test_xdr.py b/testsuite/MDAnalysisTests/coordinates/test_xdr.py index c34f307f9ab..47d65cee913 100644 --- a/testsuite/MDAnalysisTests/coordinates/test_xdr.py +++ b/testsuite/MDAnalysisTests/coordinates/test_xdr.py @@ -340,7 +340,7 @@ def test_write_trajectory(self, universe, Writer, outfile): """Test writing Gromacs trajectories (Issue 38)""" with Writer(outfile, universe.atoms.n_atoms, dt=universe.trajectory.dt) as W: for ts in universe.trajectory: - W.write_next_timestep(ts) + W.write(universe) uw = mda.Universe(GRO, outfile) @@ -366,7 +366,7 @@ def test_timestep_not_modified_by_writer(self, universe, Writer, outfile): with Writer(outfile, trj.n_atoms, dt=trj.dt) as W: # last timestep (so that time != 0) (say it again, just in case...) trj[-1] - W.write_next_timestep(ts) + W.write(universe) assert_equal( ts._pos, @@ -388,7 +388,7 @@ class TestTRRWriter(_GromacsWriter): def test_velocities(self, universe, Writer, outfile): with Writer(outfile, universe.atoms.n_atoms, dt=universe.trajectory.dt) as W: for ts in universe.trajectory: - W.write_next_timestep(ts) + W.write(universe) uw = mda.Universe(GRO, outfile) @@ -414,7 +414,7 @@ def test_gaps(self, universe, Writer, outfile): ts.has_positions = False if ts.frame % 2 == 0: ts.has_velocities = False - W.write_next_timestep(ts) + W.write(universe) uw = mda.Universe(GRO, outfile) # check that the velocities are identical for each time step, except @@ -509,7 +509,7 @@ def test_write_trajectory(self, universe, tmpdir): outfile = str(tmpdir.join('xdr-writer-issue117' + self.ext)) with mda.Writer(outfile, n_atoms=universe.atoms.n_atoms) as W: for ts in universe.trajectory: - W.write_next_timestep(ts) + W.write(universe) uw = mda.Universe(PRMncdf, outfile)