From b90e7e875de2ee4e9f6bec1c7df5e0482fb96586 Mon Sep 17 00:00:00 2001 From: Alyssa Travitz Date: Fri, 12 Dec 2025 14:23:06 -0800 Subject: [PATCH 01/10] add ligandatommapper --- gufe/mapping/__init__.py | 2 +- gufe/mapping/atom_mapper.py | 58 +++++++++++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+), 1 deletion(-) diff --git a/gufe/mapping/__init__.py b/gufe/mapping/__init__.py index 29a9bfd2f..64bda98da 100644 --- a/gufe/mapping/__init__.py +++ b/gufe/mapping/__init__.py @@ -3,7 +3,7 @@ """Defining the relationship between different components""" from .atom_mapper import AtomMapper -from .atom_mapping import AtomMapping +from .atom_mapping import AtomMapping, LigandAtomMapping from .componentmapping import ComponentMapping from .errors import AtomMappingError from .ligandatommapping import LigandAtomMapping diff --git a/gufe/mapping/atom_mapper.py b/gufe/mapping/atom_mapper.py index c28449796..4e82c06ce 100644 --- a/gufe/mapping/atom_mapper.py +++ b/gufe/mapping/atom_mapper.py @@ -6,6 +6,7 @@ import gufe from ..tokenization import GufeTokenizable +from . import LigandAtomMapping from .atom_mapping import AtomMapping @@ -26,3 +27,60 @@ def suggest_mappings(self, A: gufe.Component, B: gufe.Component) -> Iterator[Ato atom mappings between two :class:`.Component` objects. """ ... + + +class LigandAtomMapper(gufe.AtomMapper): + """ + Suggest atom mappings between two :class:`SmallMoleculeComponent` instances. + + Subclasses will typically implement the ``_mappings_generator`` method, + which returns an iterable of :class:`.LigandAtomMapping` suggestions. + """ + + @abc.abstractmethod + def _mappings_generator( + self, + componentA: SmallMoleculeComponent, + componentB: SmallMoleculeComponent, + ) -> Iterable[dict[int, int]]: + """ + Suggest mapping options for the input molecules. + + Parameters + ---------- + componentA, componentB : rdkit.Mol + the two molecules to create a mapping for + + Returns + ------- + Iterable[dict[int, int]] : + an iterable over proposed mappings from componentA to componentB + """ + ... + + def suggest_mappings( + self, + componentA: SmallMoleculeComponent, + componentB: SmallMoleculeComponent, + ) -> Iterable[LigandAtomMapping]: + """ + Suggest :class:`.LigandAtomMapping` options for the input molecules. + + Parameters + --------- + componentA, componentB : :class:`.SmallMoleculeComponent` + the two molecules to create a mapping for + + Returns + ------- + Iterable[LigandAtomMapping] : + an iterable over proposed mappings + """ + # For this base class, implementation is redundant with + # _mappings_generator. However, we keep it separate so that abstract + # subclasses of this can customize suggest_mappings while always + # maintaining the consistency that concrete implementations must + # implement _mappings_generator. + + for map_dct in self._mappings_generator(componentA, componentB): + yield LigandAtomMapping(componentA, componentB, map_dct) From d47ba91ee37234a74af9033fe0e0f267e1ce112d Mon Sep 17 00:00:00 2001 From: Alyssa Travitz Date: Fri, 12 Dec 2025 15:03:42 -0800 Subject: [PATCH 02/10] fix imports --- gufe/mapping/__init__.py | 2 +- gufe/mapping/atom_mapper.py | 58 ----------------------------- gufe/mapping/ligandatommapper.py | 63 ++++++++++++++++++++++++++++++++ 3 files changed, 64 insertions(+), 59 deletions(-) create mode 100644 gufe/mapping/ligandatommapper.py diff --git a/gufe/mapping/__init__.py b/gufe/mapping/__init__.py index 64bda98da..29a9bfd2f 100644 --- a/gufe/mapping/__init__.py +++ b/gufe/mapping/__init__.py @@ -3,7 +3,7 @@ """Defining the relationship between different components""" from .atom_mapper import AtomMapper -from .atom_mapping import AtomMapping, LigandAtomMapping +from .atom_mapping import AtomMapping from .componentmapping import ComponentMapping from .errors import AtomMappingError from .ligandatommapping import LigandAtomMapping diff --git a/gufe/mapping/atom_mapper.py b/gufe/mapping/atom_mapper.py index 4e82c06ce..c28449796 100644 --- a/gufe/mapping/atom_mapper.py +++ b/gufe/mapping/atom_mapper.py @@ -6,7 +6,6 @@ import gufe from ..tokenization import GufeTokenizable -from . import LigandAtomMapping from .atom_mapping import AtomMapping @@ -27,60 +26,3 @@ def suggest_mappings(self, A: gufe.Component, B: gufe.Component) -> Iterator[Ato atom mappings between two :class:`.Component` objects. """ ... - - -class LigandAtomMapper(gufe.AtomMapper): - """ - Suggest atom mappings between two :class:`SmallMoleculeComponent` instances. - - Subclasses will typically implement the ``_mappings_generator`` method, - which returns an iterable of :class:`.LigandAtomMapping` suggestions. - """ - - @abc.abstractmethod - def _mappings_generator( - self, - componentA: SmallMoleculeComponent, - componentB: SmallMoleculeComponent, - ) -> Iterable[dict[int, int]]: - """ - Suggest mapping options for the input molecules. - - Parameters - ---------- - componentA, componentB : rdkit.Mol - the two molecules to create a mapping for - - Returns - ------- - Iterable[dict[int, int]] : - an iterable over proposed mappings from componentA to componentB - """ - ... - - def suggest_mappings( - self, - componentA: SmallMoleculeComponent, - componentB: SmallMoleculeComponent, - ) -> Iterable[LigandAtomMapping]: - """ - Suggest :class:`.LigandAtomMapping` options for the input molecules. - - Parameters - --------- - componentA, componentB : :class:`.SmallMoleculeComponent` - the two molecules to create a mapping for - - Returns - ------- - Iterable[LigandAtomMapping] : - an iterable over proposed mappings - """ - # For this base class, implementation is redundant with - # _mappings_generator. However, we keep it separate so that abstract - # subclasses of this can customize suggest_mappings while always - # maintaining the consistency that concrete implementations must - # implement _mappings_generator. - - for map_dct in self._mappings_generator(componentA, componentB): - yield LigandAtomMapping(componentA, componentB, map_dct) diff --git a/gufe/mapping/ligandatommapper.py b/gufe/mapping/ligandatommapper.py new file mode 100644 index 000000000..1e216baf5 --- /dev/null +++ b/gufe/mapping/ligandatommapper.py @@ -0,0 +1,63 @@ +from typing import Iterable + +from gufe import SmallMoleculeComponent + +from .atom_mapper import AtomMapper +from .ligandatommapping import LigandAtomMapping + + +class LigandAtomMapper(AtomMapper): + """ + Suggest atom mappings between two :class:`SmallMoleculeComponent` instances. + + Subclasses will typically implement the ``_mappings_generator`` method, + which returns an iterable of :class:`.LigandAtomMapping` suggestions. + """ + + @abc.abstractmethod + def _mappings_generator( + self, + componentA: SmallMoleculeComponent, + componentB: SmallMoleculeComponent, + ) -> Iterable[dict[int, int]]: + """ + Suggest mapping options for the input molecules. + + Parameters + ---------- + componentA, componentB : rdkit.Mol + the two molecules to create a mapping for + + Returns + ------- + Iterable[dict[int, int]] : + an iterable over proposed mappings from componentA to componentB + """ + ... + + def suggest_mappings( + self, + componentA: SmallMoleculeComponent, + componentB: SmallMoleculeComponent, + ) -> Iterable[LigandAtomMapping]: + """ + Suggest :class:`.LigandAtomMapping` options for the input molecules. + + Parameters + --------- + componentA, componentB : :class:`.SmallMoleculeComponent` + the two molecules to create a mapping for + + Returns + ------- + Iterable[LigandAtomMapping] : + an iterable over proposed mappings + """ + # For this base class, implementation is redundant with + # _mappings_generator. However, we keep it separate so that abstract + # subclasses of this can customize suggest_mappings while always + # maintaining the consistency that concrete implementations must + # implement _mappings_generator. + + for map_dct in self._mappings_generator(componentA, componentB): + yield LigandAtomMapping(componentA, componentB, map_dct) From a71f4dd4a8ec4149bfbca9f7be492e15371f74e3 Mon Sep 17 00:00:00 2001 From: Alyssa Travitz Date: Mon, 15 Dec 2025 15:52:04 -0800 Subject: [PATCH 03/10] fix types --- gufe/mapping/ligandatommapper.py | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/gufe/mapping/ligandatommapper.py b/gufe/mapping/ligandatommapper.py index 1e216baf5..fff53d640 100644 --- a/gufe/mapping/ligandatommapper.py +++ b/gufe/mapping/ligandatommapper.py @@ -1,4 +1,5 @@ -from typing import Iterable +import abc +from typing import Iterator from gufe import SmallMoleculeComponent @@ -11,7 +12,7 @@ class LigandAtomMapper(AtomMapper): Suggest atom mappings between two :class:`SmallMoleculeComponent` instances. Subclasses will typically implement the ``_mappings_generator`` method, - which returns an iterable of :class:`.LigandAtomMapping` suggestions. + which returns an Iterator of :class:`.LigandAtomMapping` suggestions. """ @abc.abstractmethod @@ -19,7 +20,7 @@ def _mappings_generator( self, componentA: SmallMoleculeComponent, componentB: SmallMoleculeComponent, - ) -> Iterable[dict[int, int]]: + ) -> Iterator[dict[int, int]]: """ Suggest mapping options for the input molecules. @@ -30,8 +31,8 @@ def _mappings_generator( Returns ------- - Iterable[dict[int, int]] : - an iterable over proposed mappings from componentA to componentB + Iterator[dict[int, int]] : + an Iterator over proposed mappings from componentA to componentB """ ... @@ -39,7 +40,7 @@ def suggest_mappings( self, componentA: SmallMoleculeComponent, componentB: SmallMoleculeComponent, - ) -> Iterable[LigandAtomMapping]: + ) -> Iterator[LigandAtomMapping]: """ Suggest :class:`.LigandAtomMapping` options for the input molecules. @@ -50,8 +51,8 @@ def suggest_mappings( Returns ------- - Iterable[LigandAtomMapping] : - an iterable over proposed mappings + Iterator[LigandAtomMapping] : + an Iterator over proposed mappings """ # For this base class, implementation is redundant with # _mappings_generator. However, we keep it separate so that abstract From 69e8b397d862e945da304211656f021d1bc0873a Mon Sep 17 00:00:00 2001 From: Alyssa Travitz Date: Tue, 16 Dec 2025 14:06:10 -0800 Subject: [PATCH 04/10] drop abc inheritance from ComponentMapping --- gufe/mapping/componentmapping.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/gufe/mapping/componentmapping.py b/gufe/mapping/componentmapping.py index 8c665a0a6..bea4378d9 100644 --- a/gufe/mapping/componentmapping.py +++ b/gufe/mapping/componentmapping.py @@ -1,12 +1,11 @@ # This code is part of gufe and is licensed under the MIT license. # For details, see https://github.com/OpenFreeEnergy/gufe -import abc import gufe from gufe.tokenization import GufeTokenizable -class ComponentMapping(GufeTokenizable, abc.ABC): +class ComponentMapping(GufeTokenizable): """A relationship between two Components stating that they transform in some way For components that are atom-based is specialised to :class:`.AtomMapping` From 78d750cd134e98cba69f8302aeb3d348a34bb131 Mon Sep 17 00:00:00 2001 From: Alyssa Travitz Date: Thu, 18 Dec 2025 12:07:29 -0800 Subject: [PATCH 05/10] add ignores --- gufe/mapping/ligandatommapper.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/gufe/mapping/ligandatommapper.py b/gufe/mapping/ligandatommapper.py index fff53d640..6fa80a5e9 100644 --- a/gufe/mapping/ligandatommapper.py +++ b/gufe/mapping/ligandatommapper.py @@ -38,8 +38,9 @@ def _mappings_generator( def suggest_mappings( self, - componentA: SmallMoleculeComponent, - componentB: SmallMoleculeComponent, + # TODO: fix overrides when we move to min python 3.12 - see https://peps.python.org/pep-0695/#summary-examples + componentA: SmallMoleculeComponent, # type: ignore[override] + componentB: SmallMoleculeComponent, # type: ignore[override] ) -> Iterator[LigandAtomMapping]: """ Suggest :class:`.LigandAtomMapping` options for the input molecules. From 5f910c89e7f0a0d54dbf67a41d6b97412ec161e0 Mon Sep 17 00:00:00 2001 From: Alyssa Travitz Date: Thu, 18 Dec 2025 12:29:05 -0800 Subject: [PATCH 06/10] expose LigandAtomMapper alongside AtomMapper --- gufe/__init__.py | 1 + gufe/mapping/__init__.py | 1 + 2 files changed, 2 insertions(+) diff --git a/gufe/__init__.py b/gufe/__init__.py index b3c7798ef..66fd04bde 100644 --- a/gufe/__init__.py +++ b/gufe/__init__.py @@ -11,6 +11,7 @@ AtomMapper, # more specific to atom based components AtomMapping, ComponentMapping, # how individual Components relate + LigandAtomMapper, LigandAtomMapping, ) from .network import AlchemicalNetwork diff --git a/gufe/mapping/__init__.py b/gufe/mapping/__init__.py index 29a9bfd2f..7ddc46952 100644 --- a/gufe/mapping/__init__.py +++ b/gufe/mapping/__init__.py @@ -6,4 +6,5 @@ from .atom_mapping import AtomMapping from .componentmapping import ComponentMapping from .errors import AtomMappingError +from .ligandatommapper import LigandAtomMapper from .ligandatommapping import LigandAtomMapping From fbfcac342d8d455bbfff67c09bbfd6bf916d5e3f Mon Sep 17 00:00:00 2001 From: Alyssa Travitz Date: Thu, 18 Dec 2025 12:53:48 -0800 Subject: [PATCH 07/10] copy over tests from openfe --- gufe/tests/test_atommapper.py | 84 +++++++++++++++++++++++++++++++++++ 1 file changed, 84 insertions(+) create mode 100644 gufe/tests/test_atommapper.py diff --git a/gufe/tests/test_atommapper.py b/gufe/tests/test_atommapper.py new file mode 100644 index 000000000..0b9fc67da --- /dev/null +++ b/gufe/tests/test_atommapper.py @@ -0,0 +1,84 @@ +import pytest +from rdkit import Chem + +from gufe import LigandAtomMapper, LigandAtomMapping, SmallMoleculeComponent + + +def mol_from_smiles(smiles: str) -> Chem.Mol: + m = Chem.MolFromSmiles(smiles) + Chem.AllChem.Compute2DCoords(m) # type: ignore[attr-defined] + + return m + + +@pytest.fixture(scope="session") +def simple_mapping(): + """Disappearing oxygen on end + + C C O + + C C + """ + molA = SmallMoleculeComponent(mol_from_smiles("CCO")) + molB = SmallMoleculeComponent(mol_from_smiles("CC")) + + m = LigandAtomMapping(molA, molB, componentA_to_componentB={0: 0, 1: 1}) + + return m + + +@pytest.fixture(scope="session") +def other_mapping(): + """Disappearing middle carbon + + C C O + + C C + """ + molA = SmallMoleculeComponent(mol_from_smiles("CCO")) + molB = SmallMoleculeComponent(mol_from_smiles("CC")) + + m = LigandAtomMapping(molA, molB, componentA_to_componentB={0: 0, 2: 1}) + + return m + + +class TestAtomMapper: + def test_abstract_error(self, simple_mapping): + # suggest_mappings should fail with NotImplementedError if the user + # tries to directly user the abstract class + molA = simple_mapping.componentA + molB = simple_mapping.componentB + with pytest.raises(TypeError): + mapper = LigandAtomMapper() + list(mapper.suggest_mappings(molA, molB)) + + def test_concrete_mapper(self, simple_mapping, other_mapping): + # a correctly implemented concrete atom mapping should return the + # mappings generated by the _mappings_generator + molA = simple_mapping.componentA + molB = simple_mapping.componentB + + class ConcreteLigandAtomMapper(LigandAtomMapper): + def __init__(self, mappings): + self.mappings = mappings + + @classmethod + def _defaults(cls): + return {} + + def _to_dict(self): + return {"mappings": self.mappings} + + @classmethod + def _from_dict(cls, dct): + return cls(**dct) + + def _mappings_generator(self, componentA, componentB): + for mapping in self.mappings: + yield mapping.componentA_to_componentB + + mapper = ConcreteLigandAtomMapper([simple_mapping, other_mapping]) + results = list(mapper.suggest_mappings(molA, molB)) + assert len(results) == 2 + assert results == [simple_mapping, other_mapping] From 265f5cc9ee3b2448b1f4645bc34400c05c22f103 Mon Sep 17 00:00:00 2001 From: Alyssa Travitz Date: Thu, 18 Dec 2025 13:05:19 -0800 Subject: [PATCH 08/10] types --- gufe/tests/test_atommapper.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/gufe/tests/test_atommapper.py b/gufe/tests/test_atommapper.py index 0b9fc67da..bed0c2aa5 100644 --- a/gufe/tests/test_atommapper.py +++ b/gufe/tests/test_atommapper.py @@ -12,7 +12,7 @@ def mol_from_smiles(smiles: str) -> Chem.Mol: @pytest.fixture(scope="session") -def simple_mapping(): +def simple_mapping() -> LigandAtomMapping: """Disappearing oxygen on end C C O @@ -28,7 +28,7 @@ def simple_mapping(): @pytest.fixture(scope="session") -def other_mapping(): +def other_mapping() -> LigandAtomMapping: """Disappearing middle carbon C C O From 2da3fc20528dd88c7cdef611bb47708772026aab Mon Sep 17 00:00:00 2001 From: Alyssa Travitz Date: Thu, 18 Dec 2025 13:06:22 -0800 Subject: [PATCH 09/10] rename for clarity --- gufe/tests/test_atommapper.py | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/gufe/tests/test_atommapper.py b/gufe/tests/test_atommapper.py index bed0c2aa5..d4b95d172 100644 --- a/gufe/tests/test_atommapper.py +++ b/gufe/tests/test_atommapper.py @@ -43,23 +43,19 @@ def other_mapping() -> LigandAtomMapping: return m -class TestAtomMapper: - def test_abstract_error(self, simple_mapping): - # suggest_mappings should fail with NotImplementedError if the user - # tries to directly user the abstract class - molA = simple_mapping.componentA - molB = simple_mapping.componentB +class TestLigandAtomMapper: + def test_abstract_error(self): + # make sure users cannot use LigandAtomMapper directly with pytest.raises(TypeError): - mapper = LigandAtomMapper() - list(mapper.suggest_mappings(molA, molB)) + LigandAtomMapper() def test_concrete_mapper(self, simple_mapping, other_mapping): - # a correctly implemented concrete atom mapping should return the + # a correctly implemented ligand atom mapping should return the # mappings generated by the _mappings_generator molA = simple_mapping.componentA molB = simple_mapping.componentB - class ConcreteLigandAtomMapper(LigandAtomMapper): + class SimpleLigandAtomMapper(LigandAtomMapper): def __init__(self, mappings): self.mappings = mappings @@ -78,7 +74,7 @@ def _mappings_generator(self, componentA, componentB): for mapping in self.mappings: yield mapping.componentA_to_componentB - mapper = ConcreteLigandAtomMapper([simple_mapping, other_mapping]) + mapper = SimpleLigandAtomMapper([simple_mapping, other_mapping]) results = list(mapper.suggest_mappings(molA, molB)) assert len(results) == 2 assert results == [simple_mapping, other_mapping] From 6d7561316b78e18157030c9b4a31271d37af80ee Mon Sep 17 00:00:00 2001 From: Alyssa Travitz Date: Thu, 18 Dec 2025 13:22:02 -0800 Subject: [PATCH 10/10] clean up test --- gufe/tests/test_atommapper.py | 55 ++++++++++++++++++++--------------- 1 file changed, 32 insertions(+), 23 deletions(-) diff --git a/gufe/tests/test_atommapper.py b/gufe/tests/test_atommapper.py index d4b95d172..d74748275 100644 --- a/gufe/tests/test_atommapper.py +++ b/gufe/tests/test_atommapper.py @@ -3,6 +3,8 @@ from gufe import LigandAtomMapper, LigandAtomMapping, SmallMoleculeComponent +from .test_tokenization import GufeTokenizableTestsMixin + def mol_from_smiles(smiles: str) -> Chem.Mol: m = Chem.MolFromSmiles(smiles) @@ -43,38 +45,45 @@ def other_mapping() -> LigandAtomMapping: return m -class TestLigandAtomMapper: +class ExampleLigandAtomMapper(LigandAtomMapper): + def __init__(self, mappings): + self.mappings = mappings + + @classmethod + def _defaults(cls): + return {} + + def _to_dict(self): + return {"mappings": self.mappings} + + @classmethod + def _from_dict(cls, dct): + return cls(**dct) + + def _mappings_generator(self, componentA, componentB): + for mapping in self.mappings: + yield mapping.componentA_to_componentB + + +class TestLigandAtomMapper(GufeTokenizableTestsMixin): + cls = ExampleLigandAtomMapper + repr = None + + @pytest.fixture + def instance(self, simple_mapping, other_mapping): + return ExampleLigandAtomMapper([simple_mapping, other_mapping]) + def test_abstract_error(self): # make sure users cannot use LigandAtomMapper directly with pytest.raises(TypeError): LigandAtomMapper() - def test_concrete_mapper(self, simple_mapping, other_mapping): + def test_suggest_mappings(self, instance, simple_mapping, other_mapping): # a correctly implemented ligand atom mapping should return the # mappings generated by the _mappings_generator molA = simple_mapping.componentA molB = simple_mapping.componentB - class SimpleLigandAtomMapper(LigandAtomMapper): - def __init__(self, mappings): - self.mappings = mappings - - @classmethod - def _defaults(cls): - return {} - - def _to_dict(self): - return {"mappings": self.mappings} - - @classmethod - def _from_dict(cls, dct): - return cls(**dct) - - def _mappings_generator(self, componentA, componentB): - for mapping in self.mappings: - yield mapping.componentA_to_componentB - - mapper = SimpleLigandAtomMapper([simple_mapping, other_mapping]) - results = list(mapper.suggest_mappings(molA, molB)) + results = list(instance.suggest_mappings(molA, molB)) assert len(results) == 2 assert results == [simple_mapping, other_mapping]