Lightweight serialization of AtomGroups by reutilization of built Universes

package/CHANGELOG

@@ -10,11 +10,15 @@ The rules for this file:
     use tabs but use spaces for formatting
 
 ------------------------------------------------------------------------------
-??/??/15  tyler.je.reddy, richardjgowers, alejob, orbeckst, dotsdl
+??/??/15  tyler.je.reddy, richardjgowers, alejob, orbeckst, dotsdl,
+        manuel.nuno.melo
 
   * 0.11.0
 
   Enhancements
+  * AtomGroups can now be pickled/unpickled (Issue #293)
+  * Universes can have a __del__ method (not actually added) without
+    leaking (Issue #297)
   * Added reading of DL_Poly format CONFIG and HISTORY files (Issue #298)
     These can both act as both Topology and Coordinate information
   * Timestep objects now have __eq__ method
@@ -26,6 +30,12 @@ The rules for this file:
   * ProgressMeter now outputs every *interval* number of ``update`` calls
 
   Changes
+  * A ProtoReader class intermediate between IObase and Reader was added so
+    specific Readers can be subclassed without __del__ (the ChainReader and
+    SingleFrameReader), thus preventing memleaks (Issue #312).
+  * Atoms (and all container classes thereof) are now bound to Universes
+    only via weakrefs. If Universes are not explicitly kept in scope Atoms
+    will become orphaned.
   * Timestep can now only init using an integer argument (which
     represents the number of atoms)
   * Added from_timestep and from_coordinates construction methods
@@ -40,6 +50,7 @@ The rules for this file:
   * __iter__ removed from many Readers (now use base.Reader implementation)
 
   Fixes
+  * ChainReaders no longer leak (Issue #312)
   * analysis.hbonds.HydrogenBondAnalysis performs a sanity check for
     static selections (Issue #296)
   * Fixed TRZWriter failing when passed a non TRZTimestep (Issue #302)

package/MDAnalysis/__init__.py

@@ -159,7 +159,6 @@
 
 import logging
 
-
 # see the advice on logging and libraries in
 # http://docs.python.org/library/logging.html?#configuring-logging-for-a-library
 class NullHandler(logging.Handler):
@@ -246,3 +245,7 @@ class StreamWarning(Warning):
 from coordinates.core import writer as Writer
 
 collection = Timeseries.TimeseriesCollection()
+import weakref
+_anchor_universes = weakref.WeakSet()
+_named_anchor_universes = weakref.WeakSet()
+del weakref

package/MDAnalysis/coordinates/__init__.py

@@ -367,8 +367,6 @@
 
  ``close()``
      close the file and cease I/O
- ``__del__()``
-     ensure that the trajectory is closed
  ``next()``
      advance to next time step or raise :exc:`IOError` when moving
      past the last frame
@@ -379,6 +377,12 @@
  ``__exit__()``
      exit method of a `Context Manager`_, should call ``close()``.
 
+.. Note::
+   a ``__del__()`` method should also be present to ensure that the
+   trajectory is properly closed. However, certain types of Reader can ignore
+   this requirement. These include the :class:`SingleFrameReader` (file reading
+   is done within a context manager and needs no closing by hand) and the :class:`ChainReader`
+   (it is a collection of Readers, each already with its own ``__del__`` method).
 
 **Optional methods**
 

package/MDAnalysis/coordinates/base.py

@@ -602,13 +602,19 @@ def __exit__(self, exc_type, exc_val, exc_tb):
         return False  # do not suppress exceptions
 
 
-class Reader(IObase):
-    """Base class for trajectory readers.
+class ProtoReader(IObase):
+    """Base class for Readers, without a :meth:`__del__` method.
+
+    Extends :class:`IObase` with most attributes and methods of a generic Reader,
+    with the exception of a :meth:`__del__` method. It should be used as base for Readers
+    that do not need :meth:`__del__`, especially since having even an empty :meth:`__del__`
+    might lead to memory leaks.
 
     See the :ref:`Trajectory API` definition in
     :mod:`MDAnalysis.coordinates.__init__` for the required attributes and methods.
+    .. versionadded:: 0.11.0
+    .. SeeAlso:: :class:`Reader`
     """
-
     #: The appropriate Timestep class, e.g.
     #: :class:`MDAnalysis.coordinates.xdrfile.XTC.Timestep` for XTC.
     _Timestep = Timestep
@@ -686,9 +692,6 @@ def _read_next_timestep(self, ts=None):
         #     return ts
         raise NotImplementedError("BUG: Override _read_next_timestep() in the trajectory reader!")
 
-    def __del__(self):
-        self.close()
-
     def __iter__(self):
         self._reopen()
         while True:
@@ -802,8 +805,24 @@ def __repr__(self):
         return "< %s %r with %d frames of %d atoms (%d fixed) >" % \
                (self.__class__.__name__, self.filename, self.numframes, self.numatoms, self.fixed)
 
+class Reader(ProtoReader):
+    """Base class for trajectory readers that extends :class:`ProtoReader` with a :meth:`__del__` method.
 
-class ChainReader(Reader):
+    New Readers should subclass :class:`Reader` and properly implement a :meth:`close`
+    method, to ensure proper release of resources (mainly file handles). Readers that
+    are inherently safe in this regard should subclass :class:`ProtoReader` instead.
+
+    See the :ref:`Trajectory API` definition in
+    :mod:`MDAnalysis.coordinates.__init__` for the required attributes and methods.
+    .. SeeAlso:: :class:`ProtoReader`
+    .. versionchanged:: 0.11.0
+       Most of the base Reader class definitions were offloaded to :class:`ProtoReader`
+       so as to allow the subclassing of Readers without a :meth:`__del__` method.
+    """
+    def __del__(self):
+        self.close()
+
+class ChainReader(ProtoReader):
     """Reader that concatenates multiple trajectories on the fly.
 
     **Known issues**
@@ -1160,7 +1179,7 @@ def has_valid_coordinates(self, criteria, x):
 
         # def write_next_timestep(self, ts=None)
 
-class SingleFrameReader(Reader):
+class SingleFrameReader(ProtoReader):
     """Base class for Readers that only have one frame.
 
     .. versionadded:: 0.10.0
@@ -1207,11 +1226,9 @@ def _read_frame(self, frame):
 
     def read_next_timestep(self):
         raise IOError(self._err.format(self.__class__.__name__))
-                             
+
     def close(self):
         # all single frame readers should use context managers to access
-        # self.filename
+        # self.filename. Explicitly setting it to the null action in case
+        # the IObase.close method is ever changed from that.
         pass
-
-    def __del__(self):
-        self.close()