Update canary script, automation (#114)

* Update canary script

* Re-format with `ruff`

* Also run CI on Python 3.12

Author: mattwthompson

.github/workflows/CI.yml

@@ -24,6 +24,7 @@ jobs:
         python-version:
           - "3.10"
           - "3.11"
+          - "3.12"
         os:
           - ubuntu-latest
           - macos-latest

.github/workflows/canary.yaml

@@ -18,6 +18,7 @@ jobs:
         python-version:
           - "3.10"
           - "3.11"
+          - "3.12"
         cfg:
           - os: ubuntu-latest
           - os: macOS-latest

.pre-commit-config.yaml

@@ -7,20 +7,8 @@ repos:
   - id: check-yaml
   - id: trailing-whitespace
   - id: debug-statements
-- repo: https://github.com/psf/black
-  rev: 24.4.2
+- repo: https://github.com/astral-sh/ruff-pre-commit
+  rev: v0.6.4
   hooks:
-  - id: black
-- repo: https://github.com/PyCQA/isort
-  rev: 5.13.2
-  hooks:
-  - id: isort
-- repo: https://github.com/PyCQA/flake8
-  rev: 7.1.0
-  hooks:
-  - id: flake8
-- repo: https://github.com/asottile/pyupgrade
-  rev: v3.16.0
-  hooks:
-  - id: pyupgrade
-    args: ["--py310-plus"]
+    - id: ruff
+    - id: ruff-format

canary/scripts/test_hmr.py

@@ -2,10 +2,10 @@
 import sys
 
 import numpy as np
+import openmm.app
+import openmm.unit
 from openff.toolkit.topology import Molecule
 from openmmforcefields.generators import SystemGenerator
-from simtk import openmm, unit
-from simtk.openmm import app
 
 DATA_PATH = importlib.resources.files("canary") / "data"
 coverage_mols = DATA_PATH / "coverage.smi"
@@ -36,15 +36,13 @@ def hmr_driver(mol, ff_name):
     """Given an OpenFF Molecule, run a short 4 fs HMR simulation. This function is adapted from
     https://github.com/openforcefield/openforcefields/issues/19#issuecomment-689816995
     """
-    print(
-        f"Running HMR with force field {ff_name} and molecule with SMILES {mol.to_smiles()}"
-    )
+    print(f"Running HMR with force field {ff_name} and molecule with SMILES {mol.to_smiles()}")
 
     forcefield_kwargs = {
-        "constraints": app.HBonds,
+        "constraints": openmm.app.HBonds,
         "rigidWater": True,
         "removeCMMotion": False,
-        "hydrogenMass": 4 * unit.amu,
+        "hydrogenMass": 4 * openmm.unit.amu,
     }
 
     system_generator = SystemGenerator(
@@ -54,9 +52,9 @@ def hmr_driver(mol, ff_name):
     )
     system = system_generator.create_system(mol.to_topology().to_openmm())
 
-    temperature = 300 * unit.kelvin
-    collision_rate = 1.0 / unit.picoseconds
-    timestep = 4.0 * unit.femtoseconds
+    temperature = 300 * openmm.unit.kelvin
+    collision_rate = 1.0 / openmm.unit.picoseconds
+    timestep = 4.0 * openmm.unit.femtoseconds
 
     integrator = openmm.LangevinMiddleIntegrator(temperature, collision_rate, timestep)
     context = openmm.Context(system, integrator)

devtools/conda-envs/canary_env.yaml

@@ -5,5 +5,5 @@ dependencies:
   - python
   - pip
   - pytest
-  - openff-toolkit >=0.14.3
+  - openff-toolkit >=0.16.4
   - openmmforcefields

openforcefields/tests/compare.py

@@ -20,9 +20,7 @@ def _compare(
         The second system to compare.
 
     """
-    assert (
-        system1.getNumForces() == system2.getNumForces()
-    ), f"{system1.getForces()} != {system2.getForces()}"
+    assert system1.getNumForces() == system2.getNumForces(), f"{system1.getForces()} != {system2.getForces()}"
 
     for force in system1.getForces():
         assert type(force) in [openmm.NonbondedForce]
@@ -45,17 +43,11 @@ def _compare_nonbonded_forces(
         charge1, sigma1, epsilon1 = force1.getParticleParameters(particle_index)
         charge2, sigma2, epsilon2 = force2.getParticleParameters(particle_index)
 
-        assert (
-            abs(charge1 - charge2) < tolerances["charge"]
-        ), f"{charge1} != {charge2}, {charge1 - charge2}"
+        assert abs(charge1 - charge2) < tolerances["charge"], f"{charge1} != {charge2}, {charge1 - charge2}"
 
         # Water models commonly have zero epsilon and meaningless/inconsistent values
         # of sigma. In this case, do not compare sigma values.
         if epsilon1._value * epsilon2._value != 0.0:
-            assert (
-                abs(sigma1 - sigma2) < tolerances["sigma"]
-            ), f"{sigma1} != {sigma2}, {sigma1 - sigma2}"
+            assert abs(sigma1 - sigma2) < tolerances["sigma"], f"{sigma1} != {sigma2}, {sigma1 - sigma2}"
 
-        assert (
-            abs(epsilon1 - epsilon2) < tolerances["epsilon"]
-        ), f"{epsilon1} != {epsilon2}, {epsilon1 - epsilon2}"
+        assert abs(epsilon1 - epsilon2) < tolerances["epsilon"], f"{epsilon1} != {epsilon2}, {epsilon1 - epsilon2}"

openforcefields/tests/test_water_models.py

@@ -90,13 +90,7 @@ def get_virtual_site_coordinates(
     system: openmm.System,
     conformer: Quantity,
 ) -> Quantity:
-    n_virtual_sites = len(
-        [
-            i
-            for i in range(system.getNumParticles())
-            if system.getParticleMass(i)._value == 0.0
-        ]
-    )
+    n_virtual_sites = len([i for i in range(system.getNumParticles()) if system.getParticleMass(i)._value == 0.0])
 
     coordinates = openmm.unit.Quantity(
         numpy.vstack(
@@ -133,9 +127,7 @@ def get_oxygen_virtual_site_distance(
 ) -> float:
     virtual_site_coordinates = get_virtual_site_coordinates(system, conformer)
 
-    return numpy.linalg.norm(
-        (virtual_site_coordinates[index, :] - conformer[0, :]).m_as(unit.nanometer)
-    )
+    return numpy.linalg.norm((virtual_site_coordinates[index, :] - conformer[0, :]).m_as(unit.nanometer))
 
 
 def get_out_of_plane_angle(
@@ -145,9 +137,7 @@ def get_out_of_plane_angle(
 ) -> float:
     parent, orientation1, orientation2 = conformer.m_as(unit.nanometer)
 
-    virtual_site_coordinates = get_virtual_site_coordinates(system, conformer)[
-        index
-    ].m_as(unit.nanometer)
+    virtual_site_coordinates = get_virtual_site_coordinates(system, conformer)[index].m_as(unit.nanometer)
 
     normal = numpy.cross(orientation1 - parent, orientation2 - parent)
 
@@ -461,9 +451,7 @@ def test_most_recent_version_match(water_model, pattern):
 
     maximum_version = version.Version("0.0.0")
 
-    matched_files = list(
-        filter(lambda x: re.match(pattern, x) is not None, get_available_force_fields())
-    )
+    matched_files = list(filter(lambda x: re.match(pattern, x) is not None, get_available_force_fields()))
 
     assert len(matched_files) > 0, f"Failed to match any files for pattern {pattern}!"
 
@@ -474,9 +462,7 @@ def test_most_recent_version_match(water_model, pattern):
             if found_verison > maximum_version:
                 maximum_version = found_verison
 
-    assert maximum_version > version.Version(
-        "0.0.0"
-    ), f"failed to update version of {water_model}"
+    assert maximum_version > version.Version("0.0.0"), f"failed to update version of {water_model}"
 
     maximum_version_file = f"{water_model}-{maximum_version}.offxml"
     shorthand_file = f"{water_model}.offxml"
@@ -514,9 +500,7 @@ def test_ion_parameter_assignment(water_molecule):
     )
     system = ff.create_openmm_system(off_top)
 
-    nbf = [
-        force for force in system.getForces() if type(force) is openmm.NonbondedForce
-    ][0]
+    nbf = next(force for force in system.getForces() if type(force) is openmm.NonbondedForce)
 
     sigma_tol = 1e-10 * openmm.unit.nanometer
     eps_tol = 1e-10 * openmm.unit.kilojoule_per_mole
@@ -548,14 +532,10 @@ def test_ion_parameter_assignment(water_molecule):
 
     # Ensure that this test covered all the ion parameters
     for key, parameter_was_used in ion_vdw_params_used.items():
-        assert (
-            parameter_was_used
-        ), f"The ion vdW parameter with smirks {key} was not assigned"
+        assert parameter_was_used, f"The ion vdW parameter with smirks {key} was not assigned"
 
     for key, parameter_was_used in ion_librarycharge_params_used.items():
-        assert (
-            parameter_was_used
-        ), f"The ion LibraryCharge parameter with smirks {key} was not assigned"
+        assert parameter_was_used, f"The ion LibraryCharge parameter with smirks {key} was not assigned"
 
 
 @pytest.mark.parametrize(

pyproject.toml

@@ -0,0 +1,14 @@
+[tool.ruff]
+line-length = 119
+exclude = ["versioneer.py", "openforcefields/_version.py"]
+
+[tool.ruff.lint]
+select = ["F", "E", "W", "NPY", "UP", "RUF", "I"]
+
+[tool.ruff.lint.per-file-ignores]
+"openforcefields/**/__init__.py" = ["F401"]
+"scripts/add_xe_vdw.py" = ["RUF003"]
+
+[tool.ruff.lint.isort]
+known-third-party = ["openff.toolkit", "openff.utilities", "openff.units"]
+

scripts/add_xe_vdw.py

@@ -34,9 +34,7 @@
 
 # Make sure new FF loads with most recent toolkit version
 test_sage_211 = ForceField("../openforcefields/offxml/openff-2.1.1.offxml")
-test_sage_211_uc = ForceField(
-    "../openforcefields/offxml/openff_unconstrained-2.1.1.offxml"
-)
+test_sage_211_uc = ForceField("../openforcefields/offxml/openff_unconstrained-2.1.1.offxml")
 
 
 topology = Topology.from_molecules(

scripts/get_ion_nb_params_from_ambertools_frcmod.py

@@ -33,9 +33,7 @@ def main(amber_frcmod_path):
             if line.startswith("NONBON"):
                 nonbonded = True
 
-    output_path = Path(
-        "openforcefields", "data", f"{Path(amber_frcmod_path).suffix[1:]}.csv"
-    )
+    output_path = Path("openforcefields", "data", f"{Path(amber_frcmod_path).suffix[1:]}.csv")
     pandas.DataFrame(ion_nb_params).set_index("element").to_csv(output_path)