Bring type-checking back online

.github/workflows/ci.yaml

@@ -92,7 +92,6 @@ jobs:
       run: micromamba install "foyer >=0.12.1" -c conda-forge -yq
 
     - name: Run tests
-      if: always()
       run: |
         python -m pytest $COV openff/interchange/ \
           -r fExs -n logical --durations=10 \
@@ -127,7 +126,6 @@ jobs:
         python devtools/scripts/molecule-regressions.py
 
     - name: Run mypy
-      continue-on-error: true
       if: ${{ matrix.python-version == '3.11' }}
       run: |
         # As of 01/23, JAX with mypy is too slow to use without a pre-built cache

devtools/conda-envs/test_env.yaml

@@ -7,7 +7,7 @@ dependencies:
   - numpy
   - pydantic
   # OpenFF stack
-  - openff-toolkit-base >=0.16
+  - openff-toolkit-base ~=0.16
   - openff-units
   - ambertools =23
   # Needs to be explicitly listed to not be dropped when AmberTools is removed
@@ -27,7 +27,7 @@ dependencies:
   - nbval
   - nglview
   # Drivers
-  - gromacs
+  - gromacs =2024
   - lammps >=2023.08.02
   - panedr
   # Typing

openff/interchange/_tests/unit_tests/interop/openmm/test_nonbonded.py

@@ -1,6 +1,7 @@
 import pytest
 from openff.toolkit import Molecule, unit
 from openff.utilities.testing import skip_if_missing
+from pydantic import ValidationError
 
 from openff.interchange.exceptions import UnsupportedCutoffMethodError
 
@@ -25,14 +26,20 @@ def test_reaction_field(self, sage, periodic):
         if periodic:
             interchange.box = [4, 4, 4] * unit.nanometer
             interchange["Electrostatics"].periodic_potential = "reaction-field"
-        else:
-            interchange["Electrostatics"].nonperiodic_potential = "reaction-field"
 
-        with pytest.raises(
-            UnsupportedCutoffMethodError,
-            match="Reaction field electrostatics not supported. ",
-        ):
-            interchange.to_openmm(combine_nonbonded_forces=False)
+            with pytest.raises(
+                UnsupportedCutoffMethodError,
+                match="Reaction field electrostatics not supported. ",
+            ):
+                interchange.to_openmm(combine_nonbonded_forces=False)
+
+        else:
+            # Not clear that reaction field works with periodic systems, so this can't be set
+            with pytest.raises(
+                ValidationError,
+                match="Input should be 'Coulomb', 'cutoff' or 'no-cutoff'",
+            ):
+                interchange["Electrostatics"].nonperiodic_potential = "reaction-field"
 
 
 @skip_if_missing("openmm")

openff/interchange/_tests/unit_tests/interop/openmm/test_virtual_sites.py

@@ -330,7 +330,6 @@ def test_tip5p_num_exceptions(self, water, tip5p, combine, n_molecules):
                     # Safeguard against some of the behavior seen in #919
                     for index in range(num_exceptions):
                         p1, p2, *_ = force.getExceptionParameters(index)
-                        print(p1, p2)
 
                         if sorted([p1, p2]) == [0, 3]:
                             raise Exception(

openff/interchange/common/_nonbonded.py

@@ -93,6 +93,7 @@ class ElectrostaticsCollection(_NonbondedCollection):
         "Ewald3D-ConductingBoundary",
         "cutoff",
         "no-cutoff",
+        "reaction-field",
     ] = Field(_PME)
     nonperiodic_potential: Literal["Coulomb", "cutoff", "no-cutoff"] = Field("Coulomb")
     exception_potential: Literal["Coulomb"] = Field("Coulomb")
@@ -106,7 +107,7 @@ class ElectrostaticsCollection(_NonbondedCollection):
     _charges_cached: bool = PrivateAttr(default=False)
 
     @property
-    def charges(self) -> dict[TopologyKey, Quantity]:
+    def charges(self) -> dict[TopologyKey | LibraryChargeTopologyKey, Quantity]:
         """Get the total partial charge on each atom, including virtual sites."""
         if len(self._charges) == 0 or self._charges_cached is False:
             self._charges = self._get_charges(include_virtual_sites=False)
@@ -117,7 +118,7 @@ def charges(self) -> dict[TopologyKey, Quantity]:
     def _get_charges(
         self,
         include_virtual_sites: bool = False,
-    ) -> dict[TopologyKey, Quantity]:
+    ) -> dict[TopologyKey | LibraryChargeTopologyKey, Quantity]:
         if include_virtual_sites:
             raise NotImplementedError()
 

openff/interchange/components/_packmol.py

@@ -92,7 +92,7 @@ def _check_add_positive_mass(mass_to_add):
         )
 
 
-def _check_box_shape_shape(box_shape: ArrayLike):
+def _check_box_shape_shape(box_shape: NDArray):
     """Check the .shape of the box_shape argument."""
     if box_shape.shape != (3, 3):
         raise PACKMOLValueError(
@@ -531,27 +531,28 @@ def _build_input_file(
 
 
 def _center_topology_at(
-    center_solute: bool | Literal["BOX_VECS", "ORIGIN", "BRICK"],
+    center_solute: Literal["NO", "YES", "BOX_VECS", "ORIGIN", "BRICK"],
     topology: Topology,
     box_vectors: Quantity,
     brick_size: Quantity,
 ) -> Topology:
     """Return a copy of the topology centered as requested."""
-    if isinstance(center_solute, str):
-        center_solute = center_solute.upper()
+    _center_solute = center_solute.upper()
+
     topology = Topology(topology)
 
-    if center_solute is False:
+    if _center_solute == "NO":
         return topology
-    elif center_solute in [True, "BOX_VECS"]:
+    elif _center_solute in ["YES", "BOX_VECS"]:
         new_center = box_vectors.sum(axis=0) / 2.0
-    elif center_solute == "ORIGIN":
+    elif _center_solute == "ORIGIN":
         new_center = numpy.zeros(3)
-    elif center_solute == "BRICK":
+    elif _center_solute == "BRICK":
         new_center = brick_size / 2.0
     else:
         PACKMOLValueError(
-            f"center_solute must be a bool, 'BOX_VECS', 'ORIGIN', or 'BRICK', not {center_solute!r}",
+            "center_solute must be 'NO', 'YES', 'BOX_VECS', 'ORIGIN', or 'BRICK', "
+            f"not {center_solute!r}",
         )
 
     positions = topology.get_positions()
@@ -569,7 +570,7 @@ def pack_box(
     box_vectors: Quantity | None = None,
     mass_density: Quantity | None = None,
     box_shape: ArrayLike = RHOMBIC_DODECAHEDRON,
-    center_solute: bool | Literal["BOX_VECS", "ORIGIN", "BRICK"] = False,
+    center_solute: Literal["NO", "YES", "BOX_VECS", "ORIGIN", "BRICK"] = "NO",
     working_directory: str | None = None,
     retain_working_files: bool = False,
 ) -> Topology:
@@ -609,12 +610,12 @@ def pack_box(
         <http://docs.openmm.org/latest/userguide/theory/
         05_other_features.html#periodic-boundary-conditions>`_.
     center_solute
-        How to center ``solute`` in the simulation box. If ``True``
+        How to center ``solute`` in the simulation box. If ``"YES"``
         or ``"box_vecs"``, the solute's center of geometry will be placed at
         the center of the box's parallelopiped representation. If ``"origin"``,
         the solute will centered at the origin. If ``"brick"``, the solute will
         be centered in the box's rectangular brick representation. If
-        ``False`` (the default), the solute will not be moved.
+        ``"NO"`` (the default), the solute will not be moved.
     working_directory: str, optional
         The directory in which to generate the temporary working files. If
         ``None``, a temporary one will be created.
@@ -678,7 +679,7 @@ def pack_box(
     brick_size = _compute_brick_from_box_vectors(box_vectors)
 
     # Center the solute
-    if center_solute and solute is not None:
+    if center_solute != "NO" and solute is not None:
         solute = _center_topology_at(
             center_solute,
             solute,
@@ -956,5 +957,5 @@ def solvate_topology_nonwater(
         solute=topology,
         tolerance=tolerance,
         box_vectors=box_vectors,
-        center_solute=True,
+        center_solute="YES",
     )

openff/interchange/components/_particles.py

@@ -6,7 +6,7 @@
 
 from openff.toolkit import Quantity
 
-from openff.interchange._annotations import _DistanceQuantity
+from openff.interchange._annotations import _DistanceQuantity, _Quantity
 from openff.interchange.pydantic import _BaseModel
 
 
@@ -15,15 +15,16 @@ class _VirtualSite(_BaseModel, abc.ABC):
     distance: _DistanceQuantity
     orientations: tuple[int, ...]
 
-    @abc.abstractproperty
+    @property
     def local_frame_weights(self) -> tuple[list[float], ...]:
         raise NotImplementedError()
 
+    @property
     def local_frame_positions(self) -> Quantity:
         raise NotImplementedError()
 
     @property
-    def _local_frame_coordinates(self) -> Quantity:
+    def local_frame_coordinates(self) -> _Quantity:
         """
         Return the position of this virtual site in its local frame in spherical coordinates.
 

openff/interchange/components/interchange.py

@@ -427,7 +427,7 @@ def to_openmm_system(
             hydrogen_mass=hydrogen_mass,
         )
 
-    to_openmm = to_openmm_system
+    to_openmm = to_openmm_system  # type: ignore[pydantic-field]
 
     def to_openmm_topology(
         self,
@@ -559,6 +559,8 @@ def to_pdb(self, file_path: Path | str, include_virtual_sites: bool = False):
                 "Positions are required to write a `.pdb` file but found None.",
             )
 
+        assert self.positions is not None
+
         # TODO: Simply wire `include_virtual_sites` to `to_openmm_{topology|positions}`?
         if include_virtual_sites:
             from openff.interchange.interop._virtual_sites import (

openff/interchange/components/mdconfig.py

@@ -61,7 +61,7 @@ class MDConfig(_BaseModel):
         description="The distance at which the switching function is applied",
     )
     coul_method: str = Field(
-        None,
+        "Unknown",
         description="The method used to compute pairwise electrostatic interactions",
     )
     coul_cutoff: _DistanceQuantity = Field(
@@ -137,7 +137,7 @@ def apply(self, interchange: "Interchange"):
         if "Electrostatics" in interchange.collections:
             electrostatics = interchange["Electrostatics"]
             if self.coul_method.lower() == "pme":
-                electrostatics.periodic_potential = _PME  # type: ignore[assignment]
+                electrostatics.periodic_potential = _PME
             else:
                 electrostatics.periodic_potential = self.coul_method  # type: ignore[assignment]
             electrostatics.cutoff = self.coul_cutoff