Add isort to linters

.github/pull_request_template.md

@@ -1,4 +1,5 @@
 - [ ] Tag issue being addressed
 - [ ] Add [tests](https://github.com/openforcefield/openforcefield/tree/master/openforcefield/tests)
 - [ ] Update docstrings/[documentation](https://github.com/openforcefield/openforcefield/tree/master/docs), if applicable
+- [ ] [Lint](https://open-forcefield-toolkit.readthedocs.io/en/latest/developing.html#style-guide) codebase
 - [ ] Update [changelog](https://github.com/openforcefield/openforcefield/blob/master/docs/releasehistory.rst)

.github/workflows/lint.yml

@@ -18,8 +18,12 @@ jobs:
 
       - name: Install linters
         run: |
-          pip install black
+          pip install black isort
 
       - name: Run black
         run: |
           black --check openforcefield
+
+      - name: Run isort
+        run: |
+          isort --check openforcefield

docs/developing.rst

@@ -268,14 +268,16 @@ The naming conventions of classes, functions, and variables follows `PEP8 <https
 
 We place a high priority on code cleanliness and readability, even if code could be written more compactly. For example, 15-character variable names are fine. Triply nested list comprehensions are not.
 
-The ``openforcefield`` toolkit is in the process of adopting code formatting tools ("linters") to maintain consistent style and remove the burden of adhering to these standards by hand. Currently, only one is employed:
+The ``openforcefield`` toolkit is in the process of adopting code formatting tools ("linters") to maintain consistent style and remove the burden of adhering to these standards by hand. Currently, two are employed:
 1. `Black <https://black.readthedocs.io/>`_, the uncompromising code formatter, automatically formats code with a consistent style.
+1. `isort <https://timothycrosley.github.io/isort/>`_, sorts imports
 
-There is a step in CI that uses these tool(s) to check for a consistent style. To ensure that changes follow these standards, you can install and run these tool(s) locally:
+There is a step in CI that uses these tools to check for a consistent style. To ensure that changes follow these standards, you can install and run these tools locally:
 
 .. code-block:: bash
 
-    $ conda install black -c conda-forge
+    $ conda install black isort -c conda-forge
     $ black openforcefield
+    $ isort openforcefield
 
 Anything not covered above is currently up to personal preference, but may change as new linters are added.

docs/releasehistory.rst

@@ -16,6 +16,9 @@ New features
   of :py:class:`ForceField <openforcefield.typing.engines.smirnoff.forcefield.ForceField>` and ``.TAGNAME``
   of :py:class:`ParameterHandler <openforcefield.typing.engines.smirnoff.Parameters.ParameterHandler>` as
   public attributes.
+- `PR #667 <https://github.com/openforcefield/openforcefield/pull/667>`_ and
+  `PR #681 <https://github.com/openforcefield/openforcefield/pull/681>`_ linted the codebase with
+  ``black`` and ``isort``, respectively.
 
 Behavior changed
 """"""""""""""""

openforcefield/tests/conftest.py

@@ -45,6 +45,7 @@ def untar_full_alkethoh_and_freesolv_set():
     """
     import os
     import tarfile
+
     from openforcefield.utils import get_data_file_path
 
     molecule_dir_path = get_data_file_path("molecules")

openforcefield/tests/test_chemicalenvironment.py

@@ -1,11 +1,12 @@
-from openforcefield.typing.chemistry import *
 import pytest
+
+from openforcefield.typing.chemistry import *
 from openforcefield.utils.toolkits import OPENEYE_AVAILABLE
 
 # TODO: Evaluate which tests in this file should be moved to test_toolkits
 toolkits = []
 if OPENEYE_AVAILABLE:
-    from openforcefield.utils.toolkits import RDKitToolkitWrapper, OpenEyeToolkitWrapper
+    from openforcefield.utils.toolkits import OpenEyeToolkitWrapper, RDKitToolkitWrapper
 
     toolkits.append("openeye")
     toolkits.append(OpenEyeToolkitWrapper())

openforcefield/tests/test_examples.py

@@ -16,14 +16,13 @@
 import os
 import re
 import subprocess
-import textwrap
 import tempfile
+import textwrap
 
 import pytest
 
 from openforcefield.utils import RDKIT_AVAILABLE, get_data_file_path
 
-
 # ======================================================================
 # TEST UTILITIES
 # ======================================================================

openforcefield/tests/test_forcefield.py

@@ -16,32 +16,30 @@
 
 import copy
 import os
+from tempfile import NamedTemporaryFile
 
-from simtk import openmm, unit
 import numpy as np
-from numpy.testing import assert_almost_equal
-
 import pytest
-from tempfile import NamedTemporaryFile
+from numpy.testing import assert_almost_equal
+from simtk import openmm, unit
 
-from openforcefield.utils.toolkits import (
-    OpenEyeToolkitWrapper,
-    RDKitToolkitWrapper,
-    AmberToolsToolkitWrapper,
-    ToolkitRegistry,
-    ChargeMethodUnavailableError,
-)
-from openforcefield.utils import get_data_file_path
 from openforcefield.topology import Molecule, Topology
 from openforcefield.typing.engines.smirnoff import (
     ForceField,
     IncompatibleParameterError,
+    ParameterHandler,
     SMIRNOFFSpecError,
     XMLParameterIOHandler,
-    ParameterHandler,
     get_available_force_fields,
 )
-
+from openforcefield.utils import get_data_file_path
+from openforcefield.utils.toolkits import (
+    AmberToolsToolkitWrapper,
+    ChargeMethodUnavailableError,
+    OpenEyeToolkitWrapper,
+    RDKitToolkitWrapper,
+    ToolkitRegistry,
+)
 
 # ======================================================================
 # GLOBAL CONSTANTS
@@ -1134,8 +1132,8 @@ def test_parameterize_ethanol(self, toolkit_registry, registry_description):
     @pytest.fixture()
     def create_circular_handler_dependencies(self):
         from openforcefield.typing.engines.smirnoff.parameters import (
-            BondHandler,
             AngleHandler,
+            BondHandler,
             ConstraintHandler,
         )
 
@@ -1185,6 +1183,7 @@ def test_parameterize_ethanol_handler_dependency_loop(
 
     def test_parameterize_ethanol_missing_torsion(self):
         from simtk.openmm import app
+
         from openforcefield.typing.engines.smirnoff.parameters import (
             UnassignedProperTorsionParameterException,
         )
@@ -1296,8 +1295,7 @@ def test_parameterize_ethanol_different_reference_ordering_openeye(self):
         The results of both should be identical.
         """
         toolkit_registry = ToolkitRegistry(toolkit_precedence=[OpenEyeToolkitWrapper])
-        from simtk.openmm import app
-        from simtk.openmm import XmlSerializer
+        from simtk.openmm import XmlSerializer, app
 
         forcefield = ForceField("test_forcefields/smirnoff99Frosst.offxml")
         pdbfile = app.PDBFile(get_data_file_path("systems/test_systems/1_ethanol.pdb"))
@@ -1333,8 +1331,7 @@ def test_parameterize_ethanol_different_reference_ordering_rdkit(self):
         The results of both should be identical.
         """
 
-        from simtk.openmm import app
-        from simtk.openmm import XmlSerializer
+        from simtk.openmm import XmlSerializer, app
 
         toolkit_registry = ToolkitRegistry(
             toolkit_precedence=[RDKitToolkitWrapper, AmberToolsToolkitWrapper]
@@ -1571,7 +1568,7 @@ def test_charges_from_molecule(self, toolkit_registry, registry_description):
         # Create an ethanol molecule without using a toolkit
         molecules = [create_ethanol()]
 
-        from simtk.openmm import app, NonbondedForce
+        from simtk.openmm import NonbondedForce, app
 
         file_path = get_data_file_path("test_forcefields/smirnoff99Frosst.offxml")
         forcefield = ForceField(file_path)
@@ -1621,6 +1618,7 @@ def test_charges_from_molecule(self, toolkit_registry, registry_description):
     def test_nonintegral_charge_exception(self, toolkit_registry, registry_description):
         """Test skipping charge generation and instead getting charges from the original Molecule"""
         from simtk.openmm import app
+
         from openforcefield.typing.engines.smirnoff.parameters import (
             NonintegralMoleculeChargeException,
         )
@@ -1664,7 +1662,7 @@ def test_some_charges_from_molecule(self, toolkit_registry, registry_description
         cyclohexane = create_cyclohexane()
         molecules = [ethanol, cyclohexane]
 
-        from simtk.openmm import app, NonbondedForce
+        from simtk.openmm import NonbondedForce, app
 
         file_path = get_data_file_path("test_forcefields/smirnoff99Frosst.offxml")
         forcefield = ForceField(file_path)
@@ -2440,6 +2438,7 @@ def test_library_charges_dont_parameterize_molecule_because_of_incomplete_covera
     ):
         """Fail to assign charges to a molecule because not all atoms can be assigned"""
         from simtk.openmm import NonbondedForce
+
         from openforcefield.typing.engines.smirnoff.parameters import (
             UnassignedMoleculeChargeException,
         )
@@ -2686,8 +2685,8 @@ def test_alkethoh_parameters_assignment(self, alkethoh_id):
 
         """
         from openforcefield.tests.utils import (
-            get_alkethoh_file_path,
             compare_amber_smirnoff,
+            get_alkethoh_file_path,
         )
 
         # Obtain the path to the input files.
@@ -2729,10 +2728,11 @@ def test_multi_alkethoh_parameters_assignment(self):
 
         """
         import parmed
+
         from openforcefield.tests.utils import (
-            get_alkethoh_file_path,
-            compare_system_parameters,
             compare_system_energies,
+            compare_system_parameters,
+            get_alkethoh_file_path,
         )
 
         # The AlkEthOH molecule ids to mix in the systems.
@@ -2814,8 +2814,8 @@ def test_freesolv_parameters_assignment(
 
         """
         from openforcefield.tests.utils import (
-            get_freesolv_file_path,
             compare_system_parameters,
+            get_freesolv_file_path,
         )
 
         mol2_file_path, xml_file_path = get_freesolv_file_path(
@@ -2874,15 +2874,16 @@ def test_freesolv_gbsa_energies(
         SMIRNOFF-based GBSA models match the equivalent OpenMM implementations.
         """
 
+        import parmed as pmd
+        from simtk import openmm
+        from simtk.openmm import Platform
+
         from openforcefield.tests.utils import (
-            get_freesolv_file_path,
             compare_system_energies,
             create_system_from_amber,
             get_context_potential_energy,
+            get_freesolv_file_path,
         )
-        import parmed as pmd
-        from simtk import openmm
-        from simtk.openmm import Platform
 
         mol2_file_path, _ = get_freesolv_file_path(freesolv_id, forcefield_version)
 
@@ -3073,15 +3074,16 @@ def test_freesolv_gbsa_energies(
     def test_molecule_energy_gb_no_sa(self, zero_charges, gbsa_model):
         """Test creating a GBSA system without a surface energy term, and validate its energy
         against the same system made using OpenMM's AMBER GBSA functionality"""
+        import numpy as np
+        import parmed as pmd
+        from simtk import openmm
+        from simtk.openmm import Platform
+
         from openforcefield.tests.utils import (
             compare_system_energies,
             create_system_from_amber,
             get_context_potential_energy,
         )
-        import parmed as pmd
-        from simtk import openmm
-        from simtk.openmm import Platform
-        import numpy as np
 
         # Load an arbitrary molecule from the freesolv set
         molecule = Molecule.from_file(
@@ -3543,8 +3545,8 @@ def test_read_smirnoff_spec_freesolv(
         does the job.
         """
         from openforcefield.tests.utils import (
-            get_freesolv_file_path,
             compare_system_parameters,
+            get_freesolv_file_path,
         )
 
         mol2_file_path, xml_file_path = get_freesolv_file_path(

openforcefield/tests/test_io.py

@@ -18,7 +18,6 @@
 
 from openforcefield.typing.engines.smirnoff.io import XMLParameterIOHandler
 
-
 # =============================================================================================
 # QUANTITY PARSING UTILITIES
 # =============================================================================================

openforcefield/tests/test_links.py

@@ -4,7 +4,6 @@
 
 import pytest
 
-
 # ======================================================================
 # TEST UTILITIES
 # ======================================================================

openforcefield/tests/test_molecule.py

@@ -11,6 +11,7 @@
 Only the tests applicable to that toolkit will be run.
 
 TODO:
+- Will the ToolkitWrapper allow us to pare down importing each wrapper directly?
 - Add tests comparing RDKit and OpenEye aromaticity perception
 - Right now, the test database of TestMolecule is read from mol2, requiring the OE
   toolkit. Find a different test set that RDKit can read, or make a database of
@@ -31,24 +32,22 @@
 import pytest
 from simtk import unit
 
-from openforcefield.topology.molecule import Molecule, Atom, InvalidConformerError
+from openforcefield.tests.test_forcefield import (
+    create_acetaldehyde,
+    create_benzene_no_aromatic,
+    create_cis_1_2_dichloroethene,
+    create_cyclohexane,
+    create_ethanol,
+    create_reversed_ethanol,
+)
+from openforcefield.topology.molecule import Atom, InvalidConformerError, Molecule
 from openforcefield.utils import get_data_file_path
-
-# TODO: Will the ToolkitWrapper allow us to pare that down?
 from openforcefield.utils.toolkits import (
+    AmberToolsToolkitWrapper,
     OpenEyeToolkitWrapper,
     RDKitToolkitWrapper,
-    AmberToolsToolkitWrapper,
     ToolkitRegistry,
 )
-from openforcefield.tests.test_forcefield import (
-    create_ethanol,
-    create_reversed_ethanol,
-    create_acetaldehyde,
-    create_benzene_no_aromatic,
-    create_cyclohexane,
-    create_cis_1_2_dichloroethene,
-)
 
 # =============================================================================================
 # TEST UTILITIES
@@ -1048,7 +1047,6 @@ def test_to_from_file(self, molecule, format):
         from openforcefield.utils.toolkits import UndefinedStereochemistryError
 
         # TODO: Test all file capabilities; the current test is minimal
-
         # TODO: This is only for OE. Expand to both OE and RDKit toolkits.
         # Molecules that are known to raise UndefinedStereochemistryError.
         undefined_stereo_mols = {
@@ -1085,7 +1083,6 @@ def test_to_from_oemol(self, molecule):
         # Known failures raise an UndefinedStereochemistryError, but
         # the round-trip SMILES representation with the OpenEyeToolkit
         # doesn't seem to be affected.
-
         # ZINC test set known failures.
         # known_failures = {'ZINC05964684', 'ZINC05885163', 'ZINC05543156', 'ZINC17211981',
         #                   'ZINC17312986', 'ZINC06424847', 'ZINC04963126'}
@@ -1177,7 +1174,7 @@ def test_hill_formula(self):
         molecule_file = Molecule.from_file(get_data_file_path("molecules/ethanol.sdf"))
         assert molecule_smiles.hill_formula == molecule_file.hill_formula
         # make sure the topology molecule gives the same formula
-        from openforcefield.topology.topology import TopologyMolecule, Topology
+        from openforcefield.topology.topology import Topology, TopologyMolecule
 
         topology = Topology.from_molecules(molecule_smiles)
         topmol = TopologyMolecule(molecule_smiles, topology)
@@ -1219,7 +1216,7 @@ def test_isomorphic_general(self):
         # check matching with nx.Graph with full matching
         assert ethanol.is_isomorphic_with(ethanol_reverse.to_networkx()) is True
         # check matching with a TopologyMolecule class
-        from openforcefield.topology.topology import TopologyMolecule, Topology
+        from openforcefield.topology.topology import Topology, TopologyMolecule
 
         topology = Topology.from_molecules(ethanol)
         topmol = TopologyMolecule(ethanol, topology)
@@ -2617,8 +2614,8 @@ def test_compute_partial_charges(self):
         """Test computation/retrieval of partial charges"""
         # TODO: Test only one molecule for speed?
         # TODO: Do we need to deepcopy each molecule, or is setUp called separately for each test method?
-        from simtk import unit
         import numpy as np
+        from simtk import unit
 
         # Do not modify original molecules.
         molecules = copy.deepcopy(mini_drug_bank())