WIP Issue 2043 deprecate ts write (#2110)

* deprecate Timestep argument to Writers (issue #2043)

Co-authored-by: Irfan Alibay <IAlibay@users.noreply.github.com>
Co-authored-by: Lily Wang <31115101+lilyminium@users.noreply.github.com>

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
 

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()
 

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()

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)

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)

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)

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,15 +907,15 @@ 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()
 
         # 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:

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`_.

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
         --------
         <FormatWriter>.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()

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 = {}

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: