Clean up.

Remove dead code.
Resolve "to-do"s and/or reference issue tracker.
Prepare for release.

RELEASE.txt

@@ -8,7 +8,7 @@ Once the required features in gromacs-gmxapi have been merged to its master bran
 Similarly, update the `gromacs_url` embedded in setup.py
 
 After merging the gmxapi development branch to master and before tagging the release, 
-update setup.py so that it writes a version.py file with `release = True`.
+update version.in so that it writes a version.py file with `release = True`.
 
 Once the release is tagged and any last-minute essentials are cherry-picked to the refreshed development branch,
 bump the version in the development branch in CMakeLists.txt _and_ in setup.py. If / when feature branches

docs/conf.py

@@ -15,9 +15,6 @@
 # add these directories to sys.path here. If the directory is relative to the
 # documentation root, use os.path.abspath to make it absolute, like shown here.
 #
-# todo: find location of breathe package
-import os
-import sys
 from pkg_resources import get_distribution, DistributionNotFound
 # sys.path.insert(0, os.path.abspath('.'))
 #sys.path.insert(0, '@CMAKE_INSTALL_PREFIX@/@PYMOD_INSTALL_DIR@')
@@ -42,7 +39,6 @@
 #    'breathe',
 ]
 
-# todo: need to figure out how to use breathe correctly...
 # Tell breathe about the projects
 #breathe_projects = {'gmxapi': "@GMXAPI_USERDOC_OUTPUT@/xml",
 #                    'dev': "@GMXAPI_DEVDOC_OUTPUT@/xml"}

setup.py

@@ -208,9 +208,7 @@ def build_extension(self, ext):
         # Linking is a pain because the package is relocated to the site-packages directory. We should really do this
         # in two stages.
         if build_gromacs:
-            # TODO! We need to distinguish dev branch builds from master branch builds or always build with the
-            # master branch of the dependency. For one thing, as is, this line needs to be toggled for every release.
-            gromacs_url = "https://github.com/kassonlab/gromacs-gmxapi/archive/devel.zip"
+            gromacs_url = "https://github.com/kassonlab/gromacs-gmxapi/archive/v0.0.6.zip"
             gmxapi_DIR = os.path.join(extdir, 'data/gromacs')
             if build_for_readthedocs:
                 extra_cmake_args = ['-DCMAKE_INSTALL_PREFIX=' + gmxapi_DIR,

src/gmx/__init__.py

@@ -66,7 +66,6 @@
 from __future__ import print_function
 from __future__ import unicode_literals
 
-# TODO: what should happen when ``from gmx import *``?
 __all__ = ['Status', 'System', 'get_context', 'run']
 
 # Import system facilities

src/gmx/context.py

@@ -41,10 +41,9 @@ class Context(object):
     any case, the operations allow a Context implementation to transform a work specification into a
     directed acyclic graph of schedulable work.
     """
-    # \todo Put the following to-do someplace more appropriate
-    # \todo There should be a default Context active from the first `import gmx` or at least before the first logger call.
     # The Context is the appropriate entity to own or mediate access to an appropriate logging facility,
     # but right now we are using the module-level Python logger.
+    # Reference https://github.com/kassonlab/gmxapi/issues/135
     def __init__(self, workflow=None):
         """Create new context bound to the provided workflow, if any.
 
@@ -101,8 +100,6 @@ def check_workspec(cls, workspec, raises=False):
             is_valid = False
             if raises:
                 raise exceptions.ApiError('WorkSpec must contain at least one source element')
-        for name in workspec.elements:
-            element = gmx.workflow.WorkElement.deserialize()
         return is_valid
 
 
@@ -116,17 +113,23 @@ def __enter__(self):
             raise exceptions.Error('Already running.')
         # The API runner currently has an implicit context.
         try:
-            # \todo Pass the API context implementation object to launch
+            # launch() with no arguments is deprecated.
+            # Ref: https://github.com/kassonlab/gmxapi/issues/124
             self._session = self.workflow.launch()
-            # \todo Let the session provide a reference to its parent context instance.
         except:
             self._session = None
             raise
         return self._session
 
     def __exit__(self, exception_type, exception_value, traceback):
-        """Implement Python context manager protocol."""
-        # Todo: handle exceptions.
+        """Implement Python context manager protocol.
+
+        Closing a session should not produce Python exceptions. Instead, exit
+        state is accessible through API objects like Status.
+        For evolving design points, see
+        - https://github.com/kassonlab/gmxapi/issues/41
+        - https://github.com/kassonlab/gmxapi/issues/121
+        """
         self._session.close()
         self._session = None
         return False
@@ -387,6 +390,18 @@ def work(self):
 
     @work.setter
     def work(self, work):
+        """Set `work` attribute.
+
+        Raises:
+            gmx.exceptions.ApiError: work is not compatible with schema or
+                known operations.
+            gmx.exceptions.UsageError: Context can not access operations in
+                the name space given for an Element
+            gmx.exceptions.ValueError: assignment operation cannot be performed
+                for the provided object (rhs)
+
+        For discussion on error handling, see https://github.com/kassonlab/gmxapi/issues/125
+        """
         if work is None:
             warnings.warn("A Context without a valid WorkSpec is iffy...")
             return
@@ -398,20 +413,23 @@ def work(self, work):
         elif hasattr(work, 'workspec') and isinstance(work.workspec, WorkSpec):
             workspec = work.workspec
         else:
-            raise ValueError('work argument must provide a gmx.workflow.WorkSpec.')
+            raise exceptions.ValueError('work argument must provide a gmx.workflow.WorkSpec.')
         workspec._context = self
 
         # Make sure this context knows how to run the specified work.
         for e in workspec.elements:
             element = gmx.workflow.WorkElement.deserialize(workspec.elements[e])
 
-
             if element.namespace not in {'gmxapi', 'gromacs'} and element.namespace not in self.__operations:
                 # Non-built-in namespaces are treated as modules to import.
                 try:
                     element_module = importlib.import_module(element.namespace)
                 except ImportError as e:
-                    raise exceptions.UsageError('This context does not know how to invoke {} from {}. ImportError: {}'.format(element.operation, element.namespace, e.message))
+                    raise exceptions.UsageError(
+                        'This context does not know how to invoke {} from {}. ImportError: {}'.format(
+                            element.operation,
+                            element.namespace,
+                            e.message))
 
                 # Don't leave an empty nested dictionary if we couldn't map the operation.
                 if element.namespace in self.__operations:
@@ -424,7 +442,8 @@ def work(self, work):
                         element_operation = getattr(element_module, element.operation)
                         namespace_map[element.operation] = element_operation
                     except:
-                        raise exceptions.ApiError('Operation {} not found in {}.'.format(element.operation, element.namespace))
+                        raise exceptions.ApiError('Operation {} not found in {}.'.format(element.operation,
+                                                                                         element.namespace))
                     # Set or update namespace map only if we have something to contribute.
                     self.__operations[element.namespace] = namespace_map
             else:
@@ -433,12 +452,16 @@ def work(self, work):
                 assert element.namespace in self.__operations
                 if not element.operation in self.__operations[element.namespace]:
                     if self.rank < 1:
-                        logger.error("Operation {} not found in map {}".format(element.operation, str(self.__operations)))
+                        logger.error("Operation {} not found in map {}".format(element.operation,
+                                                                               str(self.__operations)))
                     # This check should be performed when deciding if the context is appropriate for the work.
                     # If we are just going to use a try/catch block for this test, then we should differentiate
                     # this exception from those raised due to incorrect usage.
-                    # \todo Consider distinguishing API misuse from API failures.
-                    raise exceptions.ApiError('Specified work cannot be performed due to unimplemented operation {}.{}.'.format(element.namespace, element.operation))
+                    # The exception thrown here may evolve with https://github.com/kassonlab/gmxapi/issues/125
+                    raise exceptions.ApiError(
+                        'Specified work cannot be performed due to unimplemented operation {}.{}.'.format(
+                            element.namespace,
+                            element.operation))
 
         self.__work = workspec
 
@@ -734,10 +757,10 @@ def __enter__(self):
         # gmxapi::Session objects are exposed as gmx.core.MDSession and provide run() and close() methods.
         #
         # Here, I want to find the input appropriate for this rank and get an MDSession for it.
-        # \todo In the future, we should set up an object with "ports" configured and then instantiate.
         # E.g. Make a pass that allows meta-objects to bind (setting md_proxy._input_tpr and md_proxy._plugins,
         # and then call a routine implemented by each object to run whatever protocol it needs, such
         # as `system = gmx.core.from_tpr(md._input_tpr); system.add_potential(md._plugins)
+        # For future design plans, reference https://github.com/kassonlab/gmxapi/issues/65
         if self._session_ensemble_size > 0:
             # print(graph)
             logger.debug(("Launching graph {}.".format(graph.graph)))
@@ -808,14 +831,14 @@ def close(self):
 
     def __exit__(self, exception_type, exception_value, traceback):
         """Implement Python context manager protocol."""
-        # Todo: handle exceptions.
-        # \todo: we should not have a None session but rather an API-compliant Session that just has no work.
         logger.info("Exiting session on context rank {}.".format(self.rank))
         if self._session is not None:
             logger.info("Calling session.close().")
             self._session.close()
             self._session = None
         else:
+            # Note: we should not have a None session but rather an API-compliant Session that just has no work.
+            # Reference: https://github.com/kassonlab/gmxapi/issues/41
             logger.info("No _session known to context or session already closed.")
         if hasattr(self, '_session_ensemble_communicator'):
             from mpi4py import MPI
@@ -828,8 +851,11 @@ def __exit__(self, exception_type, exception_value, traceback):
             logger.debug("No ensemble subcommunicator on context rank {}.".format(self.rank))
         os.chdir(self.__initial_cwd)
         logger.info("Session closed on context rank {}.".format(self.rank))
-        # \todo Make sure session has ended on all ranks before continuing and handle final errors.
+        # Note: Since sessions running in different processes can have different work, sessions have not necessarily
+        # ended on all ranks. As a result, starting another session on the same resources could block until the
+        # resources are available.
 
+        # Python context managers return False when there were no exceptions to handle.
         return False
 
 def get_context(work=None):

src/gmx/core/export_system.cpp

@@ -70,14 +70,14 @@ void export_system(py::module &m)
             }
             else
             {
-                // Todo: Need to bind the exceptions...
+                // Note: Exception behavior is likely to change.
+                // Ref: https://github.com/kassonlab/gmxapi/issues/125
                 throw PyExc_RuntimeError;
             }
         },
         "Set a restraint potential for the system.");
 
     // Export session class
-    // \todo relocate
     // We can't completely surrender ownership to Python because other API objects may refer to it.
     py::class_<::gmxapi::Session, std::shared_ptr<::gmxapi::Session>> session(m, "MDSession");
     session.def("run", &::gmxapi::Session::run, "Run the simulation workflow");

src/gmx/system.py

@@ -58,8 +58,11 @@ def _from_file(inputrecord):
         context and create a System object. If no Context is currently bound, a
         local default context is created and bound.
 
-        TODO: clarify the file location relative to the execution context.
-        Until then, this helper method should not be part of the public
+        We are still refining the meanings of paths and file locations, and the
+        semantics of turning file objects into API objects. For reference, see
+        https://github.com/kassonlab/gmxapi/issues/96
+
+        Until resolved, this helper method should not be part of the public
         interface.
 
         Args:

src/gmx/test/test_exceptions.py

@@ -6,7 +6,7 @@
 Throwing of appropriate exceptions is tested using assertRaises
 in test for the components that throw them.
 
-\todo these tests are stale but we probably should update and use them...
+Reference: https://github.com/kassonlab/gmxapi/issues/125
 """
 
 import unittest

src/gmx/test/test_pymd.py

@@ -6,6 +6,7 @@
 # I'm increasingly thinking that the CMake-managed C++ extension module should be managed separately than the setuptools
 # primary module. Then we can just do standard things like using CTest and googletest for the more complicated stuff.
 
+import warnings
 import logging
 logging.getLogger().setLevel(logging.DEBUG)
 # create console handler
@@ -140,12 +141,13 @@ def test_plugin(caplog):
     context.add_operation(potential_element.namespace, potential_element.operation, my_plugin)
     context.work = md
 
-    # \todo swallow warning about wide MPI context
-    # \todo use pytest context managers to turn raised exceptions into assertions.
-    with context as session:
-        if context.rank == 0:
-            print(context.work)
-        session.run()
+    with warnings.catch_warnings():
+        # Swallow warning about wide MPI context
+        warnings.simplefilter("ignore")
+        with context as session:
+            if context.rank == 0:
+                print(context.work)
+            session.run()
 
 
 if __name__ == '__main__':

src/gmx/test/test_workflow.py

@@ -90,7 +90,6 @@ def test_creation(self):
         # Release 0.0.4 will mark the finalization of workspec version 0.1.
         assert gmx.version.api_is_at_least(0,0,5)
         assert workspec.version == "gmxapi_workspec_0_1"
-        # \todo better python package version checking.
     def test_methods(self):
         workspec = gmx.workflow.WorkSpec()
         assert str(workspec) is not None

src/gmx/version.in

@@ -14,8 +14,8 @@ major = @PROJECT_VERSION_MAJOR@
 minor = @PROJECT_VERSION_MINOR@
 patch = @PROJECT_VERSION_PATCH@
 
-# Todo: Release status needs to be automatically maintained.
-release = False
+# Note: this is not automatically updated. See RELEASE.txt and https://github.com/kassonlab/gmxapi/issues/152
+release = True
 
 def api_is_at_least(major_version, minor_version=0, patch_version=0):
     """Allow client to check whether installed module supports the requested API level.

src/gmx/workflow.py

@@ -253,7 +253,6 @@ def _chase_deps(self, source_set, name_list):
             name_list: name list to be expanded with dependencies and sequenced
 
         Note that source_set is a reference to an object that is modified arbitrarily.
-        .. todo:: Maybe this shouldn't be a member function, but a closure within WorkSpec.__iter__()
 
         """
         assert isinstance(source_set, set)
@@ -439,7 +438,8 @@ def __init__(self, namespace="gmxapi", operation=None, params=None, depends=()):
         else:
             raise exceptions.UsageError("Invalid argument type for operation.")
 
-        # \todo It is currently non-sensical to update any attributes after adding to a workspec, but nothing prevents it.
+        # Note: Nothing currently prevents attribute updates by assignment after adding the element to a workspec,
+        # but this protocol will be clarified with https://github.com/kassonlab/gmxapi/issues/92
         if params is None:
             self.params = {}
         elif isinstance(params, dict):
@@ -642,7 +642,8 @@ def from_tpr(input=None, **kwargs):
             arg_path = os.path.abspath(arg)
             raise exceptions.UsageError(usage + " Got {}".format(arg_path))
 
-    # \todo These are runner parameters, not MD parameters, and should be in the call to gmx.run() instead of here.
+    # Note: These are runner parameters, not MD parameters, and should be in the call to gmx.run() instead of here.
+    # Reference https://github.com/kassonlab/gmxapi/issues/95
     params = {}
     for arg_key in kwargs:
         if arg_key == 'grid' or arg_key == 'dd':