Reduce Travis verbosity and remove most LGTM issues

.travis.yml

@@ -90,8 +90,8 @@ install:
 script:
   # Run doctests only if both RDKit and OpenEye are installed.
   - if [[ "$RDKIT" == true && "$OPENEYE" == true ]];
-    then pytest -v --ignore=utilities --ignore=examples/deprecated --ignore=docs --doctest-modules --nbval-lax --cov=openforcefield;
-    else pytest -v --ignore=utilities --ignore=examples/deprecated --nbval-lax --cov=openforcefield;
+    then pytest --ignore=utilities --ignore=examples/deprecated --ignore=docs --doctest-modules --nbval-lax --cov=openforcefield;
+    else pytest --ignore=utilities --ignore=examples/deprecated --nbval-lax --cov=openforcefield;
     fi
 
 notifications:

examples/deprecated/liquid_box/create_mol2_pdb/create_mol2_pdb.py

@@ -85,14 +85,13 @@ def GenerateBox(pdbin, pdbout, box, nmol, tries):
 
     fout=open('genbox.out', 'w')
     ferr=open('genbox.err', 'w')
-    p = subprocess.Popen('%s -ci %s -o genbox.pdb -box %.3f %.3f %.3f -nmol %i -try %i'
-                         % (gmxcmd, pdbin, box, box, box, nmol, tries),
-                         shell=True, stdout=fout, stderr=ferr)
+    subprocess.Popen('%s -ci %s -o genbox.pdb -box %.3f %.3f %.3f -nmol %i -try %i'
+                     % (gmxcmd, pdbin, box, box, box, nmol, tries),
+                     shell=True, stdout=fout, stderr=ferr)
     fout.close()
     ferr.close()
     t0 = time.time()
     print("Running %s to create a solvent box..." % gmxcmd)
-    stdout, stderr = p.communicate()
     print("Time elapsed: % .3f seconds" % (time.time() - t0))
     nmol_out = 0
     for line in open('genbox.err').readlines():
@@ -131,8 +130,6 @@ def run_create_mol2_pdb(**kwargs):
     smiles_string = open(input_txt).readlines()[0].strip()
     print("The following SMILES string will be converted: %s" % smiles_string)
 
-    ofs = oechem.oemolostream()
-
     # create a new molecule
     oemol = oechem.OEGraphMol()
     # convert the SMILES string into a molecule

examples/deprecated/liquid_box/openff_simulation.py

@@ -1,11 +1,9 @@
 #!/usr/bin/env python
 
-import time 
-import numpy as np
+import time
 
 # Import OpenMM tools
 from simtk import openmm, unit
-from simtk.openmm import Platform
 from simtk.openmm.app import *
 
 # Use MDTraj to write simulation trajectories
@@ -15,7 +13,7 @@
 from openforcefield.typing.engines.smirnoff import ForceField
 # LPW: openforcefield's PME is different from openmm's PME
 from openforcefield.typing.engines.smirnoff.forcefield import PME
-from openforcefield.utils import get_data_filename, extractPositionsFromOEMol, generateTopologyFromOEMol
+from openforcefield.utils import get_data_filename
 
 # Import the OpenEye toolkit
 from openeye import oechem

examples/deprecated/mixedFF_structure/generate_mixedFF_complex.py

@@ -27,7 +27,6 @@
 #
 
 # Load the protein topology
-from openforcefield.utils import generate_protein_structure
 protein_pdb_filename = get_data_filename('proteins/T4-protein.pdb')
 protein_pdbfile = app.PDBFile(protein_pdb_filename)
 

openforcefield/topology/molecule.py

@@ -611,8 +611,8 @@ def to_dict(self):
         return vsite_dict
 
 
-    @staticmethod
-    def from_dict(vsite_dict):
+    @classmethod
+    def from_dict(cls, vsite_dict):
         """Create a virtual site from a dict representation.
 
 """
@@ -777,8 +777,8 @@ def to_dict(self):
         #vsite_dict['vsite_type'] = 'BondChargeVirtualSite'
         return vsite_dict
 
-    @staticmethod
-    def from_dict(vsite_dict):
+    @classmethod
+    def from_dict(cls, vsite_dict):
         base_dict = deepcopy(vsite_dict)
         # Make sure it's the right type of virtual site
         assert vsite_dict['vsite_type'] == "BondChargeVirtualSite"
@@ -794,6 +794,28 @@ def distance(self):
         return self._distance
 
 
+class _LonePairVirtualSite(VirtualSite):
+    """Private base class for mono/di/trivalent lone pair virtual sites."""
+
+    @classmethod
+    def from_dict(cls, vsite_dict):
+        base_dict = deepcopy(vsite_dict)
+
+        # Make sure it's the right type of virtual site
+        assert vsite_dict['vsite_type'] == cls.__name__
+        base_dict.pop('vsite_type')
+        base_dict.pop('distance')
+        base_dict.pop('out_of_plane_angle')
+        base_dict.pop('in_plane_angle')
+        vsite = super().from_dict(**base_dict)
+        vsite._distance = string_to_quantity(vsite_dict['distance'])
+        vsite._in_plane_angle = string_to_quantity(
+            vsite_dict['in_plane_angle'])
+        vsite._out_of_plane_angle = string_to_quantity(
+            vsite_dict['out_of_plane_angle'])
+        return vsite
+
+
 class MonovalentLonePairVirtualSite(VirtualSite):
     """
     A particle representing a "Monovalent Lone Pair"-type virtual site, in which the location of the charge is specified by the positions of three atoms. This is originally intended for situations like a carbonyl, and allows placement of a virtual site S at a specified distance d, in_plane_angle, and out_of_plane_angle relative to a central atom and two connected atoms.
@@ -870,23 +892,23 @@ def to_dict(self):
         #vsite_dict['vsite_type'] = 'MonovalentLonePairVirtualSite'
         return vsite_dict
 
-    @staticmethod
-    def from_dict(vsite_dict):
-        base_dict = deepcopy(vsite_dict)
+    @classmethod
+    def from_dict(cls, vsite_dict):
+        """
+        Construct a new MonovalentLonePairVirtualSite from an serialized dictionary representation.
 
-        # Make sure it's the right type of virtual site
-        assert vsite_dict['vsite_type'] == "MonovalentLonePairVirtualSite"
-        base_dict.pop('vsite_type')
-        base_dict.pop('distance')
-        base_dict.pop('out_of_plane_angle')
-        base_dict.pop('in_plane_angle')
-        vsite = super().from_dict(**base_dict)
-        vsite._distance = string_to_quantity(vsite_dict['distance'])
-        vsite._in_plane_angle = string_to_quantity(
-            vsite_dict['in_plane_angle'])
-        vsite._out_of_plane_angle = string_to_quantity(
-            vsite_dict['out_of_plane_angle'])
-        return vsite
+        Parameters
+        ----------
+        vsite_dict : dict
+            The VirtualSite to deserialize.
+
+        Returns
+        -------
+        The newly created MonovalentLonePairVirtualSite
+
+        """
+        # The function is overridden only to have a custom docstring.
+        return super().from_dict(vsite_dict)
 
     @property
     def distance(self):
@@ -974,10 +996,10 @@ def to_dict(self):
         vsite_dict['vsite_type'] = self.type
         return vsite_dict
 
-    @staticmethod
-    def from_dict(vsite_dict):
+    @classmethod
+    def from_dict(cls, vsite_dict):
         """
-        Construct a new DivalentPairVirtualSite from an serialized dictionary representation.
+        Construct a new DivalentLonePairVirtualSite from an serialized dictionary representation.
 
         Parameters
         ----------
@@ -989,22 +1011,8 @@ def from_dict(vsite_dict):
         The newly created DivalentLonePairVirtualSite
 
         """
-        base_dict = deepcopy(vsite_dict)
-
-        # Make sure it's the right type of virtual site
-        assert vsite_dict['vsite_type'] == "DivalentLonePairVirtualSite"
-        base_dict.pop('vsite_type')
-        base_dict.pop('distance')
-        base_dict.pop('out_of_plane_angle')
-        base_dict.pop('in_plane_angle')
-        vsite = super().from_dict(**base_dict)
-        vsite._distance = string_to_quantity(vsite_dict['distance'])
-        vsite._in_plane_angle = string_to_quantity(
-            vsite_dict['in_plane_angle'])
-        vsite._out_of_plane_angle = string_to_quantity(
-            vsite_dict['out_of_plane_angle'])
-
-        return vsite
+        # The function is overridden only to have a custom docstring.
+        return super().from_dict(vsite_dict)
 
     @property
     def distance(self):
@@ -1093,8 +1101,8 @@ def to_dict(self):
         vsite_dict['vsite_type'] = self.type
         return vsite_dict
 
-    @staticmethod
-    def from_dict(vsite_dict):
+    @classmethod
+    def from_dict(cls, vsite_dict):
         """
         Construct a new TrivalentPairVirtualSite from an serialized dictionary representation.
 
@@ -1109,22 +1117,8 @@ def from_dict(vsite_dict):
         The newly created TrivalentLonePairVirtualSite
 
         """
-        base_dict = deepcopy(vsite_dict)
-
-        # Make sure it's the right type of virtual site
-        assert vsite_dict['vsite_type'] == "TrivalentLonePairVirtualSite"
-        base_dict.pop('vsite_type')
-        base_dict.pop('distance')
-        base_dict.pop('out_of_plane_angle')
-        base_dict.pop('in_plane_angle')
-        vsite = super().from_dict(**base_dict)
-        vsite._distance = string_to_quantity(vsite_dict['distance'])
-        vsite._in_plane_angle = string_to_quantity(
-            vsite_dict['in_plane_angle'])
-        vsite._out_of_plane_angle = string_to_quantity(
-            vsite_dict['out_of_plane_angle'])
-
-        return vsite
+        # The function is overridden only to have a custom docstring.
+        return super().from_dict(vsite_dict)
 
     @property
     def distance(self):
@@ -2027,22 +2021,22 @@ def compute_partial_charges(self,
         """
         raise NotImplementedError
         # TODO: Implement this in a way that's compliant with SMIRNOFF's <ChargeIncrementModel> tag when the spec gets finalized
-        if isinstance(toolkit_registry, ToolkitRegistry):
-            charges = toolkit_registry.call(
-                      'compute_partial_charges_am1bcc',
-                      self,
-            )
-        elif isinstance(toolkit_registry, ToolkitWrapper):
-            toolkit = toolkit_registry
-            charges = toolkit.compute_partial_charges_am1bcc(
-                self,
-                #quantum_chemical_method=quantum_chemical_method,
-                #partial_charge_method=partial_charge_method
-            )
-        else:
-            raise InvalidToolkitError(
-                'Invalid toolkit_registry passed to compute_partial_charges_am1bcc. Expected ToolkitRegistry or ToolkitWrapper. Got  {}'
-                .format(type(toolkit_registry)))
+        # if isinstance(toolkit_registry, ToolkitRegistry):
+        #     charges = toolkit_registry.call(
+        #               'compute_partial_charges_am1bcc',
+        #               self,
+        #     )
+        # elif isinstance(toolkit_registry, ToolkitWrapper):
+        #     toolkit = toolkit_registry
+        #     charges = toolkit.compute_partial_charges_am1bcc(
+        #         self,
+        #         #quantum_chemical_method=quantum_chemical_method,
+        #         #partial_charge_method=partial_charge_method
+        #     )
+        # else:
+        #     raise InvalidToolkitError(
+        #         'Invalid toolkit_registry passed to compute_partial_charges_am1bcc. Expected ToolkitRegistry or ToolkitWrapper. Got  {}'
+        #         .format(type(toolkit_registry)))
 
     def compute_wiberg_bond_orders(self,
                                    charge_model=None,

openforcefield/topology/topology.py

@@ -29,8 +29,11 @@
 from simtk import unit
 from simtk.openmm import app
 
-from openforcefield.typing.chemistry import ChemicalEnvironment, SMIRKSParsingError
-from openforcefield.utils.toolkits import DEFAULT_AROMATICITY_MODEL, ALLOWED_AROMATICITY_MODELS, DEFAULT_FRACTIONAL_BOND_ORDER_MODEL, ALLOWED_FRACTIONAL_BOND_ORDER_MODELS, DEFAULT_CHARGE_MODEL, GLOBAL_TOOLKIT_REGISTRY, ALLOWED_CHARGE_MODELS
+from openforcefield.typing.chemistry import ChemicalEnvironment
+from openforcefield.utils.toolkits import (
+    DEFAULT_AROMATICITY_MODEL, ALLOWED_AROMATICITY_MODELS, ALLOWED_FRACTIONAL_BOND_ORDER_MODELS,
+    GLOBAL_TOOLKIT_REGISTRY, ALLOWED_CHARGE_MODELS
+)
 from openforcefield.utils.serialization import Serializable
 from openforcefield.utils import MessageException
 
@@ -991,7 +994,7 @@ def from_molecules(cls, molecules):
         """
         # Ensure that we are working with an iterable
         try:
-            some_object_iterator = iter(molecules)
+            iter(molecules)
         except TypeError as te:
             # Make iterable object
             molecules = [molecules]
@@ -1848,8 +1851,6 @@ def _to_openeye(self,
 
         """
         oe_mol = oechem.OEMol()
-        molecule_atom_to_oe_atom = {
-        }  # Mapping dictionary between Molecule atoms and oe atoms
 
         # Python set used to identify atoms that are not in protein residues
         keep = set(proteinResidues).union(dnaResidues).union(rnaResidues)

openforcefield/typing/chemistry/environment.py

@@ -21,6 +21,18 @@
 
 """
 
+__all__ = [
+    'SMIRKSMismatchError',
+    'SMIRKSParsingError',
+    'ChemicalEnvironment',
+    'AtomChemicalEnvironment',
+    'BondChemicalEnvironment',
+    'AngleChemicalEnvironment',
+    'TorsionChemicalEnvironment',
+    'ImproperChemicalEnvironment'
+]
+
+
 #==============================================================================
 # GLOBAL IMPORTS
 #==============================================================================
@@ -268,7 +280,7 @@ def asSMIRKS(self):
             smirks = self.asSMARTS()
 
             # No index specified so SMIRKS = SMARTS
-            if self.index == None:
+            if self.index is None:
                 return smirks
 
             # Add label to the end of SMARTS
@@ -823,7 +835,7 @@ def _asSMIRKS(self, initialAtom = None, neighbors = None, smarts = False):
         if len(self._graph_nodes()) == 0:
             return ""
 
-        if initialAtom == None:
+        if initialAtom is None:
             initialAtom = self.getAtoms()[0]
 
         if neighbors is None:
@@ -880,7 +892,7 @@ def selectAtom(self, descriptor = None):
         a single Atom object fitting the description
         or None if no such atom exists
         """
-        if descriptor == None:
+        if descriptor is None:
             return random.choice(self._graph_nodes())
 
         # TODO: Is there a better way to do this?
@@ -1010,11 +1022,11 @@ def addAtom(self, bondToAtom, bondORtypes= None, bondANDtypes = None,
         --------
         newAtom: atom object for the newly created atom
         """
-        if bondToAtom == None:
+        if bondToAtom is None:
             if len(self._graph_nodes()) > 0:
                 return None
             newType = newAtomIndex
-            if newType == None:
+            if newType is None:
                 newType = 0
 
             newAtom = self.Atom(newORtypes, newANDtypes, newAtomIndex, newAtomRing)
@@ -1108,7 +1120,7 @@ def getBonds(self, atom = None):
         --------
         a complete list of bonds in the fragment
         """
-        if atom == None:
+        if atom is None:
             edge_list = self._graph_edges(data=True)
             bonds = [data['bond'] for a1, a2, data in edge_list]
         else:
@@ -1236,7 +1248,7 @@ def isUnindexed(self, component):
         returns True if the atom or bond is not indexed
         """
         if component._atom:
-            return component.index == None
+            return component.index is None
         else:
             return component._bond_type < 1
 
@@ -1408,7 +1420,7 @@ def __init__(self, smirks = "[*:1]~[*:2]", label = None, replacements = None, to
 
         # Add initial atom
         self.atom2 = self.selectAtom(2)
-        if self.atom2 == None:
+        if self.atom2 is None:
             raise Exception("Error: Bonds need 2 indexed atoms, there were not enough in %s" % smirks)
 
         self.bond2 = self._graph_get_edge_data(self.atom1, self.atom2)['bond']