From 576368a8cdad6d6d70dcb15f6fe35ede1573ee5f Mon Sep 17 00:00:00 2001 From: Alyssa Travitz Date: Fri, 15 Aug 2025 13:29:41 -0700 Subject: [PATCH 01/10] deprecate get_alchemical_charge_difference --- openfe/protocols/openmm_rfe/equil_rfe_methods.py | 10 ++-------- openfe/utils/ligand_utils.py | 8 ++++---- 2 files changed, 6 insertions(+), 12 deletions(-) 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/utils/ligand_utils.py b/openfe/utils/ligand_utils.py index d20415d8e..315d4697d 100644 --- a/openfe/utils/ligand_utils.py +++ b/openfe/utils/ligand_utils.py @@ -1,5 +1,7 @@ +from warnings import deprecated from gufe import LigandAtomMapping +@deprecated("Use gufe.LigandAtomMapping.get_alchemical_charge_difference() instead") def get_alchemical_charge_difference(mapping: LigandAtomMapping) -> int: """ Return the difference in formal charge between stateA and stateB defined as (formal charge A - formal charge B) @@ -14,7 +16,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 + + return mapping.get_alchemical_charge_difference() From 8c565499d94a453c2a06e5c5dda287ad51ed5abb Mon Sep 17 00:00:00 2001 From: Alyssa Travitz Date: Fri, 15 Aug 2025 13:39:55 -0700 Subject: [PATCH 02/10] fix deprecated call within relative alchemical network planner --- .../relative_alchemical_network_planner.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) 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 " From 9213a88c35c50a6f596a9a2547c6dc2b098841b4 Mon Sep 17 00:00:00 2001 From: Alyssa Travitz Date: Fri, 15 Aug 2025 13:55:22 -0700 Subject: [PATCH 03/10] add test for deprecation --- openfe/tests/setup/atom_mapping/test_atommapper.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/openfe/tests/setup/atom_mapping/test_atommapper.py b/openfe/tests/setup/atom_mapping/test_atommapper.py index 99729ce7c..8d5045487 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="Use gufe\.LigandAtomMapping"): + ligand_utils.get_alchemical_charge_difference(simple_mapping) \ No newline at end of file From 5b9f8921b3635702538fb450312655010bca7d1d Mon Sep 17 00:00:00 2001 From: Alyssa Travitz Date: Fri, 15 Aug 2025 14:14:09 -0700 Subject: [PATCH 04/10] add news item --- news/deprecate_alchemical_charge_diff.rst | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) create mode 100644 news/deprecate_alchemical_charge_diff.rst 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:** + +* From 115d7e7ef58dd37d20ae2376fbfd5c487f142a51 Mon Sep 17 00:00:00 2001 From: Alyssa Travitz Date: Fri, 15 Aug 2025 14:57:34 -0700 Subject: [PATCH 05/10] point env to working gufe branch --- environment.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/environment.yml b/environment.yml index acd85e538..8e9c35732 100644 --- a/environment.yml +++ b/environment.yml @@ -51,4 +51,4 @@ dependencies: # Control blas/openmp threads - threadpoolctl - pip: - - git+https://github.com/OpenFreeEnergy/gufe@main + - git+https://github.com/OpenFreeEnergy/gufe@migrate_ligand_utils_from_openfe From fc0f3f25ecbf008bbf65924950da6d8cda467b2f Mon Sep 17 00:00:00 2001 From: Alyssa Travitz Date: Fri, 15 Aug 2025 14:59:53 -0700 Subject: [PATCH 06/10] fix escape sequence --- openfe/tests/setup/atom_mapping/test_atommapper.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/openfe/tests/setup/atom_mapping/test_atommapper.py b/openfe/tests/setup/atom_mapping/test_atommapper.py index 8d5045487..8fbbe93b6 100644 --- a/openfe/tests/setup/atom_mapping/test_atommapper.py +++ b/openfe/tests/setup/atom_mapping/test_atommapper.py @@ -45,5 +45,5 @@ def _mappings_generator(self, componentA, componentB): def test_alchemical_charge_deprecation_warning(self, simple_mapping): - with pytest.warns(DeprecationWarning, match="Use gufe\.LigandAtomMapping"): - ligand_utils.get_alchemical_charge_difference(simple_mapping) \ No newline at end of file + with pytest.warns(DeprecationWarning, match=r"Use gufe\.LigandAtomMapping"): + ligand_utils.get_alchemical_charge_difference(simple_mapping) From 37de1ae4be5bd30eef50f31871c43c47566970c2 Mon Sep 17 00:00:00 2001 From: Alyssa Travitz Date: Fri, 15 Aug 2025 15:31:24 -0700 Subject: [PATCH 07/10] use warnings.warn instead of decorator --- openfe/utils/ligand_utils.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/openfe/utils/ligand_utils.py b/openfe/utils/ligand_utils.py index 315d4697d..2c83494df 100644 --- a/openfe/utils/ligand_utils.py +++ b/openfe/utils/ligand_utils.py @@ -1,7 +1,6 @@ -from warnings import deprecated from gufe import LigandAtomMapping +import warnings -@deprecated("Use gufe.LigandAtomMapping.get_alchemical_charge_difference() instead") def get_alchemical_charge_difference(mapping: LigandAtomMapping) -> int: """ Return the difference in formal charge between stateA and stateB defined as (formal charge A - formal charge B) @@ -16,5 +15,5 @@ def get_alchemical_charge_difference(mapping: LigandAtomMapping) -> int: int: The difference in formal charge between the end states. """ - + warnings.warn("Use gufe.LigandAtomMapping.get_alchemical_charge_difference() instead.", DeprecationWarning) return mapping.get_alchemical_charge_difference() From 0f333dc2b2f6a3d4b165b44d41fece10a896d5f2 Mon Sep 17 00:00:00 2001 From: Alyssa Travitz Date: Mon, 18 Aug 2025 11:37:35 -0700 Subject: [PATCH 08/10] env back to main --- environment.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/environment.yml b/environment.yml index 8e9c35732..acd85e538 100644 --- a/environment.yml +++ b/environment.yml @@ -51,4 +51,4 @@ dependencies: # Control blas/openmp threads - threadpoolctl - pip: - - git+https://github.com/OpenFreeEnergy/gufe@migrate_ligand_utils_from_openfe + - git+https://github.com/OpenFreeEnergy/gufe@main From a15668e4eae58453925cb91dbc8349644a435df2 Mon Sep 17 00:00:00 2001 From: Alyssa Travitz Date: Mon, 18 Aug 2025 14:14:45 -0700 Subject: [PATCH 09/10] clear cache and bump ci to pull gufe main From 48002a5c485f920943f10f5a3cff570a1f293fa1 Mon Sep 17 00:00:00 2001 From: Alyssa Travitz Date: Mon, 18 Aug 2025 16:08:30 -0700 Subject: [PATCH 10/10] clear cache and bump ci to pull gufe main