diff --git a/news/deprecate_alchemical_charge_diff.rst b/news/deprecate_alchemical_charge_diff.rst new file mode 100644 index 000000000..ce4c5b13a --- /dev/null +++ b/news/deprecate_alchemical_charge_diff.rst @@ -0,0 +1,23 @@ +**Added:** + +* + +**Changed:** + +* + +**Deprecated:** + +* Deprecated ``openfe.utils.ligand_utils.get_alchemical_charge_difference()``, which is replaced by ``LigandAtomMapping.get_alchemical_charge_difference()`` in ``gufe`` (PR #1479). + +**Removed:** + +* + +**Fixed:** + +* + +**Security:** + +* diff --git a/openfe/protocols/openmm_rfe/equil_rfe_methods.py b/openfe/protocols/openmm_rfe/equil_rfe_methods.py index c7786f732..ea4ab2b8d 100644 --- a/openfe/protocols/openmm_rfe/equil_rfe_methods.py +++ b/openfe/protocols/openmm_rfe/equil_rfe_methods.py @@ -133,14 +133,8 @@ def _get_alchemical_charge_difference( The formal charge difference between states A and B. This is defined as sum(charge state A) - sum(charge state B) """ - chg_A = Chem.rdmolops.GetFormalCharge( - mapping.componentA.to_rdkit() - ) - chg_B = Chem.rdmolops.GetFormalCharge( - mapping.componentB.to_rdkit() - ) - - difference = chg_A - chg_B + + difference = mapping.get_alchemical_charge_difference() if abs(difference) > 0: if explicit_charge_correction: diff --git a/openfe/setup/alchemical_network_planner/relative_alchemical_network_planner.py b/openfe/setup/alchemical_network_planner/relative_alchemical_network_planner.py index 577e0c5a2..a6ac782eb 100644 --- a/openfe/setup/alchemical_network_planner/relative_alchemical_network_planner.py +++ b/openfe/setup/alchemical_network_planner/relative_alchemical_network_planner.py @@ -34,7 +34,6 @@ RFEComponentLabels, ) from ...protocols.openmm_rfe.equil_rfe_methods import RelativeHybridTopologyProtocol -from ...utils.ligand_utils import get_alchemical_charge_difference # TODO: move/or find better structure for protocol_generator combinations! PROTOCOL_GENERATOR = { @@ -331,7 +330,7 @@ def _build_transformation( protocol_settings = transformation_protocol.settings.unfrozen_copy() if "vacuum" in transformation_name: protocol_settings.forcefield_settings.nonbonded_method = "nocutoff" - elif get_alchemical_charge_difference(ligand_mapping_edge) != 0: + elif ligand_mapping_edge.get_alchemical_charge_difference() != 0: wmsg = ("Charge changing transformation between ligands " f"{ligand_mapping_edge.componentA.name} and {ligand_mapping_edge.componentB.name}. " "A more expensive protocol with 22 lambda windows, sampled " diff --git a/openfe/tests/setup/atom_mapping/test_atommapper.py b/openfe/tests/setup/atom_mapping/test_atommapper.py index 99729ce7c..8fbbe93b6 100644 --- a/openfe/tests/setup/atom_mapping/test_atommapper.py +++ b/openfe/tests/setup/atom_mapping/test_atommapper.py @@ -1,7 +1,7 @@ import pytest from openfe.setup.atom_mapping.ligandatommapper import LigandAtomMapper - +from openfe.utils import ligand_utils class TestAtomMapper: def test_abstract_error(self, simple_mapping): @@ -42,3 +42,8 @@ def _mappings_generator(self, componentA, componentB): results = list(mapper.suggest_mappings(molA, molB)) assert len(results) == 2 assert results == [simple_mapping, other_mapping] + + + def test_alchemical_charge_deprecation_warning(self, simple_mapping): + with pytest.warns(DeprecationWarning, match=r"Use gufe\.LigandAtomMapping"): + ligand_utils.get_alchemical_charge_difference(simple_mapping) diff --git a/openfe/utils/ligand_utils.py b/openfe/utils/ligand_utils.py index d20415d8e..2c83494df 100644 --- a/openfe/utils/ligand_utils.py +++ b/openfe/utils/ligand_utils.py @@ -1,4 +1,5 @@ from gufe import LigandAtomMapping +import warnings def get_alchemical_charge_difference(mapping: LigandAtomMapping) -> int: """ @@ -14,7 +15,5 @@ def get_alchemical_charge_difference(mapping: LigandAtomMapping) -> int: int: The difference in formal charge between the end states. """ - from rdkit import Chem - charge_a = Chem.rdmolops.GetFormalCharge(mapping.componentA.to_rdkit()) - charge_b = Chem.rdmolops.GetFormalCharge(mapping.componentB.to_rdkit()) - return charge_a - charge_b + warnings.warn("Use gufe.LigandAtomMapping.get_alchemical_charge_difference() instead.", DeprecationWarning) + return mapping.get_alchemical_charge_difference()