Skip to content

Commit

Permalink
FIX: Fix tests
Browse files Browse the repository at this point in the history
  • Loading branch information
mattwthompson committed Oct 3, 2024
1 parent 884b221 commit 31edc75
Show file tree
Hide file tree
Showing 7 changed files with 16 additions and 15 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ jobs:
python devtools/scripts/molecule-regressions.py
- name: Run mypy
if: ${{ matrix.python-version == '3.10' }}
if: ${{ matrix.python-version == '3.11' }}
run: |
# As of 01/23, JAX with mypy is too slow to use without a pre-built cache
# https://github.com/openforcefield/openff-interchange/pull/578#issuecomment-1369979875
Expand Down
1 change: 1 addition & 0 deletions docs/releasehistory.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ Please note that all releases prior to a version 1.0.0 are considered pre-releas
* `ProperTorsionKey` no longer accepts an empty tuple as atom indices.
* Fixes a regression in which some `ElectrostaticsCollection.charges` properties did not return cached values.
* The `charge_from_molecules` argument must include only molecules that contain partial charges and are non-isomorphic with each other.
* The `charge_from_molecules` argument as used by the OpenFF Toolkit is handled internally as `molecules_with_preset_charges`.

## 0.3.30 - 2024-08

Expand Down
4 changes: 2 additions & 2 deletions openff/interchange/_tests/unit_tests/smirnoff/test_create.py
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@ def test_charges_from_molecule_reordered(

assert numpy.allclose(expected_charges, found_charges)

def test_error_when_any_missing_partial_charges(sage, self):
def test_error_when_any_missing_partial_charges(self, sage):
topology = Topology.from_molecules(
[
Molecule.from_smiles("C"),
Expand All @@ -243,7 +243,7 @@ def test_error_when_any_missing_partial_charges(sage, self):

with pytest.raises(
PresetChargesError,
match="all.*must have partial charges",
match="All.*must have partial charges",
):
sage.create_interchange(
topology,
Expand Down
2 changes: 1 addition & 1 deletion openff/interchange/common/_nonbonded.py
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ def charges(
self,
) -> dict[TopologyKey | LibraryChargeTopologyKey | VirtualSiteKey, _ElementaryChargeQuantity]:
"""Get the total partial charge on each atom, including virtual sites."""
if len(self._charges) == 0 or self._charges_cached is False: # type: ignore[has-type]
if len(self._charges) == 0 or self._charges_cached is False:
self._charges.update(self._get_charges(include_virtual_sites=False))
self._charges_cached = True

Expand Down
2 changes: 1 addition & 1 deletion openff/interchange/components/interchange.py
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ def from_smirnoff(
topology=topology,
box=box,
positions=positions,
charge_from_molecules=charge_from_molecules,
molecules_with_preset_charges=charge_from_molecules,
partial_bond_orders_from_molecules=partial_bond_orders_from_molecules,
allow_nonintegral_charges=allow_nonintegral_charges,
)
Expand Down
2 changes: 1 addition & 1 deletion openff/interchange/foyer/_nonbonded.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ class FoyerElectrostaticsHandler(ElectrostaticsCollection):
force_field_key: str = "atoms"
cutoff: _DistanceQuantity = 9.0 * unit.angstrom

_charges: dict[TopologyKey, Quantity] = PrivateAttr(default_factory=dict) # type: ignore[assignment]
_charges: dict[TopologyKey, Quantity] = PrivateAttr(default_factory=dict)

def store_charges(
self,
Expand Down
18 changes: 9 additions & 9 deletions openff/interchange/smirnoff/_create.py
Original file line number Diff line number Diff line change
Expand Up @@ -98,11 +98,11 @@ def _preprocess_preset_charges(
molecules_with_preset_charges: list[Molecule] | None,
) -> list[Molecule] | None:
"""
Pre-process the charge_from_molecules argument.
Pre-process the molecules_with_preset_charges argument.
If charge_from_molecules is None, return None.
If molecules_with_preset_charges is None, return None.
If charge_from_molecules is list[Molecule], ensure that
If molecules_with_preset_charges is list[Molecule], ensure that
1. The input is a list of Molecules
2. Each molecule has assign partial charges
Expand All @@ -112,17 +112,17 @@ def _preprocess_preset_charges(
if molecules_with_preset_charges is None:
return None

molecule_set = {molecule.to_smiles for molecule in molecules_with_preset_charges}
molecule_set = {molecule.to_smiles() for molecule in molecules_with_preset_charges}

if len(molecule_set) != len(molecules_with_preset_charges):
raise PresetChargesError(
"All molecules in the charge_from_molecules list must be isomorphically unique from each other",
"All molecules in the molecules_with_preset_charges list must be isomorphically unique from each other",
)

for molecule in molecules_with_preset_charges:
if molecule.partial_charges is None:
raise PresetChargesError(
"All molecules in the charge_from_molecules list must have partial charges assigned.",
"All molecules in the molecules_with_preset_charges list must have partial charges assigned.",
)

return molecules_with_preset_charges
Expand All @@ -133,11 +133,11 @@ def _create_interchange(
topology: Topology | list[Molecule],
box: Quantity | None = None,
positions: Quantity | None = None,
charge_from_molecules: list[Molecule] | None = None,
molecules_with_preset_charges: list[Molecule] | None = None,
partial_bond_orders_from_molecules: list[Molecule] | None = None,
allow_nonintegral_charges: bool = False,
) -> Interchange:
molecules_with_preset_charges = _preprocess_preset_charges(charge_from_molecules)
molecules_with_preset_charges = _preprocess_preset_charges(molecules_with_preset_charges)

_check_supported_handlers(force_field)

Expand Down Expand Up @@ -341,7 +341,7 @@ def _electrostatics(
if handler is not None
],
topology=topology,
charge_from_molecules=molecules_with_preset_charges,
molecules_with_preset_charges=molecules_with_preset_charges,
allow_nonintegral_charges=allow_nonintegral_charges,
),
},
Expand Down

0 comments on commit 31edc75

Please sign in to comment.