From f116585f10b77c082d5e40a7fa531b60be20fb8e Mon Sep 17 00:00:00 2001 From: Alyssa Travitz Date: Thu, 16 Jan 2025 15:31:43 -0800 Subject: [PATCH 1/9] first pass at deprecation warning --- lomap/gufe_bindings/network_generation.py | 36 ++++++++++++++++------- lomap/tests/test_new_api.py | 14 +++++---- 2 files changed, 34 insertions(+), 16 deletions(-) diff --git a/lomap/gufe_bindings/network_generation.py b/lomap/gufe_bindings/network_generation.py index f3db6b7..9f7917a 100644 --- a/lomap/gufe_bindings/network_generation.py +++ b/lomap/gufe_bindings/network_generation.py @@ -5,10 +5,12 @@ ) import gufe import itertools +import functools import logging import networkx as nx import numpy as np -from typing import Callable, Optional, Union +from typing import Callable, Optional, Union, Any +import warnings from ..graphgen import GraphGen from .._due import due, Doi @@ -16,10 +18,24 @@ logger = logging.getLogger(__name__) +def deprecated_kwarg(old_name:str, new_name:str): + def decorator(func: Callable)->Callable: + # old_name = 'molecules' + # new_name = 'ligands' + @functools.wraps(func) + def wrapper(*args:Any, **kwargs:Any): + if old_name in kwargs: + kwargs[new_name] = kwargs.pop(old_name) + warnings.warn(f"{func.__name__} argument '{old_name}' is deprecated. Please use '{new_name}' instead.", + DeprecationWarning) + return func(*args, **kwargs) + return wrapper + return decorator +@deprecated_kwarg(old_name='molecules', new_name='ligands') @due.dcite(Doi("https://doi.org/10.1007/s10822-013-9678-y"), description="LOMAP") def generate_lomap_network( - molecules: list[gufe.SmallMoleculeComponent], + ligands: list[gufe.SmallMoleculeComponent], mappers: Union[AtomMapper, list[AtomMapper]], scorer: Callable, distance_cutoff: float = 0.4, @@ -35,7 +51,7 @@ def generate_lomap_network( Parameters ---------- - molecules : list[SmallMoleculeComponent] + ligands : list[SmallMoleculeComponent] molecules to map mappers : list[AtomMapper] or AtomMapper one or more Mapper functions to use to propose edges @@ -67,17 +83,17 @@ def generate_lomap_network( if isinstance(mappers, gufe.AtomMapper): mappers = [mappers] if actives is None: - actives = [False] * len(molecules) + actives = [False] * len(ligands) # gen n x n mappings with scores # initially all zero scores, i.e. impossible - mtx = np.zeros((len(molecules), len(molecules)), dtype=float) + mtx = np.zeros((len(ligands), len(ligands)), dtype=float) # np array of mappings mps = np.zeros_like(mtx, dtype=object) - # for all combinations of molecules - for i, j in itertools.combinations(range(len(molecules)), 2): - mA, mB = molecules[i], molecules[j] + # for all combinations of ligands + for i, j in itertools.combinations(range(len(ligands)), 2): + mA, mB = ligands[i], ligands[j] # pick best score across all mappings from all mappings best_mp: Optional[LigandAtomMapping] = None @@ -107,7 +123,7 @@ def generate_lomap_network( gg = GraphGen(score_matrix=mtx, ids=list(range(mtx.shape[0])), - names=[m.name for m in molecules], + names=[m.name for m in ligands], max_path_length=max_path_length, actives=actives, max_dist_from_active=max_dist_from_active, @@ -121,7 +137,7 @@ def generate_lomap_network( ln = LigandNetwork( edges=[mps[i, j] for i, j in n.edges], - nodes=molecules, + nodes=ligands, ) return ln diff --git a/lomap/tests/test_new_api.py b/lomap/tests/test_new_api.py index 68043da..248c466 100644 --- a/lomap/tests/test_new_api.py +++ b/lomap/tests/test_new_api.py @@ -29,10 +29,12 @@ def basic(): def test_generate_network_smoketest(basic): - network = lomap.generate_lomap_network( - molecules=basic, - mappers=lomap.LomapAtomMapper(), - scorer=lomap.default_lomap_score, - ) + with pytest.deprecated_call(): + network = lomap.generate_lomap_network( + molecules=basic, + mappers=lomap.LomapAtomMapper(), + scorer=lomap.default_lomap_score, + ) + + assert isinstance(network, gufe.LigandNetwork) - assert isinstance(network, gufe.LigandNetwork) From 92d2c4200dd0362c9f6e9fc20b44480b5e2c2165 Mon Sep 17 00:00:00 2001 From: Alyssa Travitz Date: Thu, 16 Jan 2025 15:38:35 -0800 Subject: [PATCH 2/9] using helper function --- lomap/gufe_bindings/network_generation.py | 27 ++++++++++++++++------- 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/lomap/gufe_bindings/network_generation.py b/lomap/gufe_bindings/network_generation.py index 9f7917a..47db85d 100644 --- a/lomap/gufe_bindings/network_generation.py +++ b/lomap/gufe_bindings/network_generation.py @@ -9,7 +9,7 @@ import logging import networkx as nx import numpy as np -from typing import Callable, Optional, Union, Any +from typing import Callable, Optional, Union, Any, Dict import warnings from ..graphgen import GraphGen @@ -18,16 +18,27 @@ logger = logging.getLogger(__name__) -def deprecated_kwarg(old_name:str, new_name:str): +def rename_kwargs(func_name:str, kwargs:Dict[str, Any], old_name:str, new_name:str): + """Helper function for deprecating function arguments.""" + if old_name in kwargs: + warnings.warn(f"{func_name} argument '{old_name}' is deprecated. Please use '{new_name}' instead.", + DeprecationWarning) + kwargs[new_name] = kwargs.pop(old_name) + return kwargs + +def deprecated_kwarg(old_name:str, new_name:str) -> Callable: + """Decorator for deprecating keyword arguments + + e.g. + @deprecated_kwarg(old_arg='new_arg') + def my_function(new_arg) + ... + """ def decorator(func: Callable)->Callable: - # old_name = 'molecules' - # new_name = 'ligands' + @functools.wraps(func) def wrapper(*args:Any, **kwargs:Any): - if old_name in kwargs: - kwargs[new_name] = kwargs.pop(old_name) - warnings.warn(f"{func.__name__} argument '{old_name}' is deprecated. Please use '{new_name}' instead.", - DeprecationWarning) + kwargs = rename_kwargs(func.__name__, kwargs, old_name=old_name, new_name=new_name) return func(*args, **kwargs) return wrapper return decorator From a05d7f155c1f3d8063929d43a484ca4f9273d594 Mon Sep 17 00:00:00 2001 From: Alyssa Travitz Date: Thu, 16 Jan 2025 15:42:50 -0800 Subject: [PATCH 3/9] add overdefined handling --- lomap/gufe_bindings/network_generation.py | 10 +++++++--- lomap/tests/test_new_api.py | 9 +++++++++ 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/lomap/gufe_bindings/network_generation.py b/lomap/gufe_bindings/network_generation.py index 47db85d..6432e70 100644 --- a/lomap/gufe_bindings/network_generation.py +++ b/lomap/gufe_bindings/network_generation.py @@ -21,16 +21,20 @@ def rename_kwargs(func_name:str, kwargs:Dict[str, Any], old_name:str, new_name:str): """Helper function for deprecating function arguments.""" if old_name in kwargs: - warnings.warn(f"{func_name} argument '{old_name}' is deprecated. Please use '{new_name}' instead.", + if new_name in kwargs: + raise ValueError(f"Both {new_name} and {old_name} are defined for {func_name}. {old_name} is deprecated, please use {new_name} instead.") + + else: + warnings.warn(f"{func_name} argument '{old_name}' is deprecated. Please use '{new_name}' instead.", DeprecationWarning) - kwargs[new_name] = kwargs.pop(old_name) + kwargs[new_name] = kwargs.pop(old_name) return kwargs def deprecated_kwarg(old_name:str, new_name:str) -> Callable: """Decorator for deprecating keyword arguments e.g. - @deprecated_kwarg(old_arg='new_arg') + @deprecated_kwarg(old_arg, new_arg) def my_function(new_arg) ... """ diff --git a/lomap/tests/test_new_api.py b/lomap/tests/test_new_api.py index 248c466..7d108d9 100644 --- a/lomap/tests/test_new_api.py +++ b/lomap/tests/test_new_api.py @@ -38,3 +38,12 @@ def test_generate_network_smoketest(basic): assert isinstance(network, gufe.LigandNetwork) + +def test_overdefined_generate_network(basic): + with pytest.raises(ValueError): + lomap.generate_lomap_network( + molecules=basic, + ligands=basic, + mappers=lomap.LomapAtomMapper(), + scorer=lomap.default_lomap_score, + ) From 133970841f98c6fdafe100e3e7c48b2e04c48faf Mon Sep 17 00:00:00 2001 From: Alyssa Travitz Date: Fri, 17 Jan 2025 09:35:45 -0800 Subject: [PATCH 4/9] moving deprecation function into utils --- lomap/gufe_bindings/network_generation.py | 37 ++----------------- lomap/utils.py | 45 +++++++++++++++++++++++ 2 files changed, 48 insertions(+), 34 deletions(-) create mode 100644 lomap/utils.py diff --git a/lomap/gufe_bindings/network_generation.py b/lomap/gufe_bindings/network_generation.py index 6432e70..c9edc9c 100644 --- a/lomap/gufe_bindings/network_generation.py +++ b/lomap/gufe_bindings/network_generation.py @@ -5,49 +5,18 @@ ) import gufe import itertools -import functools import logging import networkx as nx import numpy as np -from typing import Callable, Optional, Union, Any, Dict -import warnings +from typing import Callable, Optional, Union from ..graphgen import GraphGen from .._due import due, Doi - +from ..utils import deprecated_kwargs logger = logging.getLogger(__name__) -def rename_kwargs(func_name:str, kwargs:Dict[str, Any], old_name:str, new_name:str): - """Helper function for deprecating function arguments.""" - if old_name in kwargs: - if new_name in kwargs: - raise ValueError(f"Both {new_name} and {old_name} are defined for {func_name}. {old_name} is deprecated, please use {new_name} instead.") - - else: - warnings.warn(f"{func_name} argument '{old_name}' is deprecated. Please use '{new_name}' instead.", - DeprecationWarning) - kwargs[new_name] = kwargs.pop(old_name) - return kwargs - -def deprecated_kwarg(old_name:str, new_name:str) -> Callable: - """Decorator for deprecating keyword arguments - - e.g. - @deprecated_kwarg(old_arg, new_arg) - def my_function(new_arg) - ... - """ - def decorator(func: Callable)->Callable: - - @functools.wraps(func) - def wrapper(*args:Any, **kwargs:Any): - kwargs = rename_kwargs(func.__name__, kwargs, old_name=old_name, new_name=new_name) - return func(*args, **kwargs) - return wrapper - return decorator - -@deprecated_kwarg(old_name='molecules', new_name='ligands') +@deprecated_kwargs(name_mappings={'molecules':'ligands'}) @due.dcite(Doi("https://doi.org/10.1007/s10822-013-9678-y"), description="LOMAP") def generate_lomap_network( ligands: list[gufe.SmallMoleculeComponent], diff --git a/lomap/utils.py b/lomap/utils.py new file mode 100644 index 0000000..0ffb953 --- /dev/null +++ b/lomap/utils.py @@ -0,0 +1,45 @@ +import functools +import warnings +from typing import Any, Callable, Dict + + +def rename_kwargs(func_name: str, kwargs: Dict[str, Any], name_mappings: Dict[str, str]): + """Helper function for deprecating function arguments.""" + for old_name, new_name in name_mappings.items(): + if old_name in kwargs: + if new_name in kwargs: + raise ValueError( + f"Both {new_name} and {old_name} are defined for {func_name}." + + f"{old_name} is deprecated, please use {new_name} instead." + ) + + else: + warnings.warn( + f"{func_name} argument '{old_name}' is deprecated. Please use '{new_name}' instead.", + DeprecationWarning, + ) + kwargs[new_name] = kwargs.pop(old_name) + return kwargs + + +def deprecated_kwargs(name_mappings: Dict[str, str]) -> Callable: + # TODO: make this work for multiple args + """Decorator for deprecating keyword arguments + + e.g. + @deprecated_kwarg({'old_arg':'new_arg'}) + def my_function(new_arg) + ... + """ + + def decorator(func: Callable) -> Callable: + @functools.wraps(func) + def wrapper(*args: Any, **kwargs: Any): + kwargs = rename_kwargs( + func_name=func.__name__, kwargs=kwargs, name_mappings=name_mappings + ) + return func(*args, **kwargs) + + return wrapper + + return decorator From 2fbc2e37b517c1b616eb30f268bc3c340402a81f Mon Sep 17 00:00:00 2001 From: Alyssa Travitz Date: Fri, 17 Jan 2025 09:36:39 -0800 Subject: [PATCH 5/9] clearer test func name --- lomap/tests/test_new_api.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lomap/tests/test_new_api.py b/lomap/tests/test_new_api.py index 7d108d9..c4b2e48 100644 --- a/lomap/tests/test_new_api.py +++ b/lomap/tests/test_new_api.py @@ -39,7 +39,7 @@ def test_generate_network_smoketest(basic): assert isinstance(network, gufe.LigandNetwork) -def test_overdefined_generate_network(basic): +def test_overdefined_deprecated_generate_network(basic): with pytest.raises(ValueError): lomap.generate_lomap_network( molecules=basic, From e91b2c8b4b9ff93ccbb956b690936376f10cfc64 Mon Sep 17 00:00:00 2001 From: Alyssa Travitz Date: Fri, 17 Jan 2025 09:38:21 -0800 Subject: [PATCH 6/9] remove completed TODO --- lomap/utils.py | 1 - 1 file changed, 1 deletion(-) diff --git a/lomap/utils.py b/lomap/utils.py index 0ffb953..10c3fed 100644 --- a/lomap/utils.py +++ b/lomap/utils.py @@ -23,7 +23,6 @@ def rename_kwargs(func_name: str, kwargs: Dict[str, Any], name_mappings: Dict[st def deprecated_kwargs(name_mappings: Dict[str, str]) -> Callable: - # TODO: make this work for multiple args """Decorator for deprecating keyword arguments e.g. From 1bf74c202aa406976773292f21018dc69a2a08db Mon Sep 17 00:00:00 2001 From: Alyssa Travitz Date: Fri, 17 Jan 2025 14:20:17 -0800 Subject: [PATCH 7/9] updating changelog --- CHANGELOG | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG b/CHANGELOG index 2d46f5b..cadd9b4 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -2,6 +2,10 @@ CHANGELOG ********* +Unreleased +========== +- Replaced the ``molecules`` argument with ``ligands`` in ``generate_lomap_network()``. Argument name ``molecules`` will be deprecated. + 2.3.0 ===== - Added shift option to MCS and cli (-s or --shift), this defaults to True. In combination with threed/-3 this option From 7ff156d711a093f3dcb6a0f1ed909cc826b0a9f7 Mon Sep 17 00:00:00 2001 From: Alyssa Travitz Date: Thu, 23 Jan 2025 11:33:20 -0800 Subject: [PATCH 8/9] fixing rename_kwargs so that it handles multiple args --- lomap/utils.py | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/lomap/utils.py b/lomap/utils.py index 10c3fed..b03585e 100644 --- a/lomap/utils.py +++ b/lomap/utils.py @@ -3,23 +3,25 @@ from typing import Any, Callable, Dict -def rename_kwargs(func_name: str, kwargs: Dict[str, Any], name_mappings: Dict[str, str]): +def rename_kwargs( + func_name: str, kwargs: Dict[str, Any], name_mappings: Dict[str, str] +): """Helper function for deprecating function arguments.""" for old_name, new_name in name_mappings.items(): + deprecation_msg = ( + f"{func_name} argument '{old_name}' is deprecated, please use '{new_name}' instead.", + ) if old_name in kwargs: if new_name in kwargs: raise ValueError( - f"Both {new_name} and {old_name} are defined for {func_name}." - + f"{old_name} is deprecated, please use {new_name} instead." + f"Both '{new_name}' and '{old_name}' are defined for {func_name}." + + f"'{old_name}' is deprecated, please use '{new_name}' instead." ) else: - warnings.warn( - f"{func_name} argument '{old_name}' is deprecated. Please use '{new_name}' instead.", - DeprecationWarning, - ) + warnings.warn(deprecation_msg, DeprecationWarning) kwargs[new_name] = kwargs.pop(old_name) - return kwargs + return kwargs def deprecated_kwargs(name_mappings: Dict[str, str]) -> Callable: From 5e200188246f56ba67a71f399a3c383ff6d70642 Mon Sep 17 00:00:00 2001 From: Alyssa Travitz Date: Thu, 23 Jan 2025 11:35:05 -0800 Subject: [PATCH 9/9] adding match to pytest raises --- lomap/tests/test_new_api.py | 4 ++-- lomap/utils.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lomap/tests/test_new_api.py b/lomap/tests/test_new_api.py index c4b2e48..a2f4550 100644 --- a/lomap/tests/test_new_api.py +++ b/lomap/tests/test_new_api.py @@ -29,7 +29,7 @@ def basic(): def test_generate_network_smoketest(basic): - with pytest.deprecated_call(): + with pytest.deprecated_call(match="'molecules' is deprecated, please use 'ligands'"): network = lomap.generate_lomap_network( molecules=basic, mappers=lomap.LomapAtomMapper(), @@ -40,7 +40,7 @@ def test_generate_network_smoketest(basic): def test_overdefined_deprecated_generate_network(basic): - with pytest.raises(ValueError): + with pytest.raises(ValueError, match="Both 'molecules' and 'ligands' are defined"): lomap.generate_lomap_network( molecules=basic, ligands=basic, diff --git a/lomap/utils.py b/lomap/utils.py index b03585e..4f4f66a 100644 --- a/lomap/utils.py +++ b/lomap/utils.py @@ -14,7 +14,7 @@ def rename_kwargs( if old_name in kwargs: if new_name in kwargs: raise ValueError( - f"Both '{new_name}' and '{old_name}' are defined for {func_name}." + f"Both '{old_name}' and '{new_name}' are defined for {func_name}." + f"'{old_name}' is deprecated, please use '{new_name}' instead." )