From 6c629dcc30de564fc6189ccd65d296419b92d469 Mon Sep 17 00:00:00 2001 From: Alyssa Travitz Date: Thu, 23 Jan 2025 12:39:52 -0800 Subject: [PATCH 1/7] switch warning to log.info --- gufe/components/explicitmoleculecomponent.py | 12 ++++++++---- gufe/tests/test_smallmoleculecomponent.py | 20 ++++++++++++-------- 2 files changed, 20 insertions(+), 12 deletions(-) diff --git a/gufe/components/explicitmoleculecomponent.py b/gufe/components/explicitmoleculecomponent.py index f360f765..919dd95e 100644 --- a/gufe/components/explicitmoleculecomponent.py +++ b/gufe/components/explicitmoleculecomponent.py @@ -1,4 +1,5 @@ import json +import logging import warnings from typing import Optional @@ -34,7 +35,7 @@ def _ensure_ofe_name(mol: RDKitMol, name: str) -> str: return name -def _check_partial_charges(mol: RDKitMol) -> None: +def _check_partial_charges(mol: RDKitMol, logger=None) -> None: """ Checks for the presence of partial charges. @@ -88,9 +89,12 @@ def _check_partial_charges(mol: RDKitMol) -> None: wmsg = ( "Partial charges have been provided, these will " "preferentially be used instead of generating new " - "partial charges" + "partial charges." ) - warnings.warn(wmsg) + if logger is None: + logger = logging.getLogger() + + logger.info(wmsg) class ExplicitMoleculeComponent(Component): @@ -106,7 +110,7 @@ class ExplicitMoleculeComponent(Component): def __init__(self, rdkit: RDKitMol, name: str = ""): name = _ensure_ofe_name(rdkit, name) - _check_partial_charges(rdkit) + _check_partial_charges(rdkit, logger=self.logger) conformers = list(rdkit.GetConformers()) if not conformers: raise ValueError("Molecule was provided with no conformers.") diff --git a/gufe/tests/test_smallmoleculecomponent.py b/gufe/tests/test_smallmoleculecomponent.py index 838ba472..f15aa4f5 100644 --- a/gufe/tests/test_smallmoleculecomponent.py +++ b/gufe/tests/test_smallmoleculecomponent.py @@ -14,6 +14,7 @@ import json import os from unittest import mock +import logging import pytest from rdkit import Chem @@ -247,10 +248,11 @@ def charged_off_ethane(self, ethane): off_ethane.assign_partial_charges(partial_charge_method="am1bcc") return off_ethane - def test_partial_charges_warning(self, charged_off_ethane): - matchmsg = "Partial charges have been provided" - with pytest.warns(UserWarning, match=matchmsg): - SmallMoleculeComponent.from_openff(charged_off_ethane) + def test_partial_charges_logging(self, charged_off_ethane, caplog): + caplog.set_level(logging.INFO) + SmallMoleculeComponent.from_openff(charged_off_ethane) + + assert "Partial charges have been provided" in caplog.text def test_partial_charges_zero_warning(self, charged_off_ethane): charged_off_ethane.partial_charges[:] = 0 * unit.elementary_charge @@ -271,7 +273,7 @@ def test_partial_charges_too_few_atoms(self): with pytest.raises(ValueError, match="Incorrect number of"): SmallMoleculeComponent.from_rdkit(mol) - def test_partial_charges_applied_to_atoms(self): + def test_partial_charges_applied_to_atoms(self, caplog): """ Make sure that charges set at the molecule level are transferred to atoms and picked up by openFF. @@ -280,9 +282,11 @@ def test_partial_charges_applied_to_atoms(self): Chem.AllChem.Compute2DCoords(mol) # add some fake charges at the molecule level mol.SetProp("atom.dprop.PartialCharge", "-1 0.25 0.25 0.25 0.25") - matchmsg = "Partial charges have been provided" - with pytest.warns(UserWarning, match=matchmsg): - ofe = SmallMoleculeComponent.from_rdkit(mol) + caplog.set_level(logging.INFO) + + ofe = SmallMoleculeComponent.from_rdkit(mol) + assert "Partial charges have been provided" in caplog.text + # convert to openff and make sure the charges are set off_mol = ofe.to_openff() assert off_mol.partial_charges is not None From 9cbe5b511daf133feaa53227566a2a39a47ab515 Mon Sep 17 00:00:00 2001 From: Alyssa Travitz Date: Thu, 23 Jan 2025 12:42:51 -0800 Subject: [PATCH 2/7] clearer var name --- gufe/components/explicitmoleculecomponent.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/gufe/components/explicitmoleculecomponent.py b/gufe/components/explicitmoleculecomponent.py index 919dd95e..c4747d60 100644 --- a/gufe/components/explicitmoleculecomponent.py +++ b/gufe/components/explicitmoleculecomponent.py @@ -86,7 +86,7 @@ def _check_partial_charges(mol: RDKitMol, logger=None) -> None: wmsg = f"Partial charges provided all equal to " "zero. These may be ignored by some Protocols." warnings.warn(wmsg) else: - wmsg = ( + message = ( "Partial charges have been provided, these will " "preferentially be used instead of generating new " "partial charges." @@ -94,7 +94,7 @@ def _check_partial_charges(mol: RDKitMol, logger=None) -> None: if logger is None: logger = logging.getLogger() - logger.info(wmsg) + logger.info(message) class ExplicitMoleculeComponent(Component): From b6569c2d2a09b84fabf78961ae3c8fda25b324c7 Mon Sep 17 00:00:00 2001 From: Alyssa Travitz Date: Thu, 23 Jan 2025 16:15:49 -0800 Subject: [PATCH 3/7] adding test for check_partial_charges being called without logging --- gufe/components/explicitmoleculecomponent.py | 2 +- gufe/tests/test_explicitmoleculecomponent.py | 1 - gufe/tests/test_smallmoleculecomponent.py | 10 ++++++++-- 3 files changed, 9 insertions(+), 4 deletions(-) diff --git a/gufe/components/explicitmoleculecomponent.py b/gufe/components/explicitmoleculecomponent.py index c4747d60..e7417684 100644 --- a/gufe/components/explicitmoleculecomponent.py +++ b/gufe/components/explicitmoleculecomponent.py @@ -83,7 +83,7 @@ def _check_partial_charges(mol: RDKitMol, logger=None) -> None: raise ValueError(errmsg) if np.all(np.isclose(p_chgs, 0.0)): - wmsg = f"Partial charges provided all equal to " "zero. These may be ignored by some Protocols." + wmsg = "Partial charges provided all equal to zero. These may be ignored by some Protocols." warnings.warn(wmsg) else: message = ( diff --git a/gufe/tests/test_explicitmoleculecomponent.py b/gufe/tests/test_explicitmoleculecomponent.py index 002e4135..e851945c 100644 --- a/gufe/tests/test_explicitmoleculecomponent.py +++ b/gufe/tests/test_explicitmoleculecomponent.py @@ -1,6 +1,5 @@ import pickle - class ExplicitMoleculeComponentMixin: def test_pickle(self, instance): diff --git a/gufe/tests/test_smallmoleculecomponent.py b/gufe/tests/test_smallmoleculecomponent.py index f15aa4f5..932b7a39 100644 --- a/gufe/tests/test_smallmoleculecomponent.py +++ b/gufe/tests/test_smallmoleculecomponent.py @@ -22,7 +22,7 @@ import gufe from gufe import SmallMoleculeComponent -from gufe.components.explicitmoleculecomponent import _ensure_ofe_name +from gufe.components.explicitmoleculecomponent import _ensure_ofe_name, _check_partial_charges from gufe.tokenization import TOKENIZABLE_REGISTRY from .test_explicitmoleculecomponent import ExplicitMoleculeComponentMixin @@ -75,7 +75,6 @@ def test_ensure_ofe_name(internal, rdkit_name, name, expected, recwarn): assert out_name == expected assert rdkit.GetProp("ofe-name") == out_name - class TestSmallMoleculeComponent(GufeTokenizableTestsMixin, ExplicitMoleculeComponentMixin): cls = SmallMoleculeComponent @@ -248,6 +247,13 @@ def charged_off_ethane(self, ethane): off_ethane.assign_partial_charges(partial_charge_method="am1bcc") return off_ethane + def test_check_partial_charges_without_gufe_logger(self, charged_off_ethane, caplog): + rd_mol = charged_off_ethane.to_rdkit() + caplog.set_level(logging.INFO) + _check_partial_charges(rd_mol, logger=None) + assert "Partial charges have been provided" in caplog.text + + def test_partial_charges_logging(self, charged_off_ethane, caplog): caplog.set_level(logging.INFO) SmallMoleculeComponent.from_openff(charged_off_ethane) From ad989cdee9bcaf51596f5e5d6a4847303376d1ba Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Fri, 24 Jan 2025 16:00:33 +0000 Subject: [PATCH 4/7] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- gufe/tests/test_explicitmoleculecomponent.py | 1 + gufe/tests/test_smallmoleculecomponent.py | 6 +++--- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/gufe/tests/test_explicitmoleculecomponent.py b/gufe/tests/test_explicitmoleculecomponent.py index e851945c..002e4135 100644 --- a/gufe/tests/test_explicitmoleculecomponent.py +++ b/gufe/tests/test_explicitmoleculecomponent.py @@ -1,5 +1,6 @@ import pickle + class ExplicitMoleculeComponentMixin: def test_pickle(self, instance): diff --git a/gufe/tests/test_smallmoleculecomponent.py b/gufe/tests/test_smallmoleculecomponent.py index 932b7a39..5d2ed7ea 100644 --- a/gufe/tests/test_smallmoleculecomponent.py +++ b/gufe/tests/test_smallmoleculecomponent.py @@ -12,9 +12,9 @@ else: HAS_OFFTK = True import json +import logging import os from unittest import mock -import logging import pytest from rdkit import Chem @@ -22,7 +22,7 @@ import gufe from gufe import SmallMoleculeComponent -from gufe.components.explicitmoleculecomponent import _ensure_ofe_name, _check_partial_charges +from gufe.components.explicitmoleculecomponent import _check_partial_charges, _ensure_ofe_name from gufe.tokenization import TOKENIZABLE_REGISTRY from .test_explicitmoleculecomponent import ExplicitMoleculeComponentMixin @@ -75,6 +75,7 @@ def test_ensure_ofe_name(internal, rdkit_name, name, expected, recwarn): assert out_name == expected assert rdkit.GetProp("ofe-name") == out_name + class TestSmallMoleculeComponent(GufeTokenizableTestsMixin, ExplicitMoleculeComponentMixin): cls = SmallMoleculeComponent @@ -253,7 +254,6 @@ def test_check_partial_charges_without_gufe_logger(self, charged_off_ethane, cap _check_partial_charges(rd_mol, logger=None) assert "Partial charges have been provided" in caplog.text - def test_partial_charges_logging(self, charged_off_ethane, caplog): caplog.set_level(logging.INFO) SmallMoleculeComponent.from_openff(charged_off_ethane) From 7c9e63d24941eb8809f043daa71a1bce0755acfb Mon Sep 17 00:00:00 2001 From: Alyssa Travitz <31974495+atravitz@users.noreply.github.com> Date: Fri, 24 Jan 2025 10:26:30 -0800 Subject: [PATCH 5/7] Update gufe/components/explicitmoleculecomponent.py Co-authored-by: David L. Dotson --- gufe/components/explicitmoleculecomponent.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gufe/components/explicitmoleculecomponent.py b/gufe/components/explicitmoleculecomponent.py index e7417684..d6226848 100644 --- a/gufe/components/explicitmoleculecomponent.py +++ b/gufe/components/explicitmoleculecomponent.py @@ -92,7 +92,7 @@ def _check_partial_charges(mol: RDKitMol, logger=None) -> None: "partial charges." ) if logger is None: - logger = logging.getLogger() + logger = logging.getLogger(__name__) logger.info(message) From 3cf59ac1bb91973a5f783a730b834859b664d0bf Mon Sep 17 00:00:00 2001 From: Alyssa Travitz Date: Wed, 29 Jan 2025 16:17:26 -0800 Subject: [PATCH 6/7] output SMC name when available --- gufe/components/explicitmoleculecomponent.py | 12 ++++++------ gufe/tests/test_smallmoleculecomponent.py | 6 +++--- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/gufe/components/explicitmoleculecomponent.py b/gufe/components/explicitmoleculecomponent.py index d6226848..38a40b8d 100644 --- a/gufe/components/explicitmoleculecomponent.py +++ b/gufe/components/explicitmoleculecomponent.py @@ -86,15 +86,15 @@ def _check_partial_charges(mol: RDKitMol, logger=None) -> None: wmsg = "Partial charges provided all equal to zero. These may be ignored by some Protocols." warnings.warn(wmsg) else: - message = ( - "Partial charges have been provided, these will " - "preferentially be used instead of generating new " - "partial charges." - ) + msg = "Partial charges have been provided" + if name:= mol.GetProp("ofe-name"): + msg+= f" for {name}" + msg+=", these will preferentially be used instead of generating new partial charges." + if logger is None: logger = logging.getLogger(__name__) - logger.info(message) + logger.info(msg) class ExplicitMoleculeComponent(Component): diff --git a/gufe/tests/test_smallmoleculecomponent.py b/gufe/tests/test_smallmoleculecomponent.py index 5d2ed7ea..574fcf61 100644 --- a/gufe/tests/test_smallmoleculecomponent.py +++ b/gufe/tests/test_smallmoleculecomponent.py @@ -243,8 +243,8 @@ def test_to_off_name(self, named_ethane): @pytest.mark.skipif(not HAS_OFFTK, reason="no openff tookit available") class TestSmallMoleculeComponentPartialCharges: @pytest.fixture(scope="function") - def charged_off_ethane(self, ethane): - off_ethane = ethane.to_openff() + def charged_off_ethane(self, named_ethane): + off_ethane = named_ethane.to_openff() off_ethane.assign_partial_charges(partial_charge_method="am1bcc") return off_ethane @@ -252,7 +252,7 @@ def test_check_partial_charges_without_gufe_logger(self, charged_off_ethane, cap rd_mol = charged_off_ethane.to_rdkit() caplog.set_level(logging.INFO) _check_partial_charges(rd_mol, logger=None) - assert "Partial charges have been provided" in caplog.text + assert "Partial charges have been provided for ethane, these" in caplog.text def test_partial_charges_logging(self, charged_off_ethane, caplog): caplog.set_level(logging.INFO) From 309352f0bea8f716e18574da64da8cb4ca76d775 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Thu, 30 Jan 2025 15:42:39 +0000 Subject: [PATCH 7/7] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- gufe/components/explicitmoleculecomponent.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/gufe/components/explicitmoleculecomponent.py b/gufe/components/explicitmoleculecomponent.py index 38a40b8d..e0a66baf 100644 --- a/gufe/components/explicitmoleculecomponent.py +++ b/gufe/components/explicitmoleculecomponent.py @@ -87,9 +87,9 @@ def _check_partial_charges(mol: RDKitMol, logger=None) -> None: warnings.warn(wmsg) else: msg = "Partial charges have been provided" - if name:= mol.GetProp("ofe-name"): - msg+= f" for {name}" - msg+=", these will preferentially be used instead of generating new partial charges." + if name := mol.GetProp("ofe-name"): + msg += f" for {name}" + msg += ", these will preferentially be used instead of generating new partial charges." if logger is None: logger = logging.getLogger(__name__)