From d6bc933909b429aad98269b0ce9d19ac5cfd96e5 Mon Sep 17 00:00:00 2001 From: CalCraven Date: Thu, 10 Aug 2023 10:46:11 -0500 Subject: [PATCH 01/11] deprecate any foyer to gmso conversion that does not use forcefield utilities. Updates tests to load ff from XML through ff-utils --- environment-dev.yml | 2 +- environment.yml | 2 +- gmso/core/forcefield.py | 28 +++++++++++++-------- gmso/external/__init__.py | 1 - gmso/external/convert_foyer_xml.py | 4 +++ gmso/tests/base_test.py | 25 +++---------------- gmso/tests/test_convert_foyer_xml.py | 37 ++++++++++------------------ gmso/tests/test_forcefield.py | 6 +++-- gmso/utils/decorators.py | 14 +++++++++++ gmso/utils/ff_utils.py | 8 ++++-- 10 files changed, 65 insertions(+), 62 deletions(-) diff --git a/environment-dev.yml b/environment-dev.yml index e46e3b35b..0286bbb67 100644 --- a/environment-dev.yml +++ b/environment-dev.yml @@ -9,7 +9,7 @@ dependencies: - unyt<=2.9.2 - boltons - lxml - - pydantic>1.8,<2.0 + - pydantic=1.10.11 - networkx - pytest - mbuild>=0.11.0 diff --git a/environment.yml b/environment.yml index 0046582ea..c7ef337f2 100644 --- a/environment.yml +++ b/environment.yml @@ -9,7 +9,7 @@ dependencies: - unyt<=2.9.2 - boltons - lxml - - pydantic>1.8,<2.0 + - pydantic=1.10.11 - networkx - ele>=0.2.0 - foyer>=0.11.3 diff --git a/gmso/core/forcefield.py b/gmso/core/forcefield.py index e7f86699b..5c25c2063 100644 --- a/gmso/core/forcefield.py +++ b/gmso/core/forcefield.py @@ -9,9 +9,13 @@ from lxml import etree from gmso.core.element import element_by_symbol -from gmso.exceptions import GMSOError, MissingPotentialError +from gmso.exceptions import ( + ForceFieldParseError, + GMSOError, + MissingPotentialError, +) from gmso.utils._constants import FF_TOKENS_SEPARATOR -from gmso.utils.decorators import deprecate_kwargs +from gmso.utils.decorators import deprecate_function, deprecate_kwargs from gmso.utils.ff_utils import ( parse_ff_atomtypes, parse_ff_connection_types, @@ -45,10 +49,8 @@ class ForceField(object): Parameters ---------- - name : str - Name of the forcefield, default 'ForceField' - version : str - a cannonical semantic version of the forcefield, default 1.0.0 + xml_loc : str + Path to the forcefield xml. The forcefield xml can be either in Foyer or GMSO style. strict: bool, default=True If true, perform a strict validation of the forcefield XML file greedy: bool, default=True @@ -104,6 +106,7 @@ def __init__( "ffutils", ]: ff = ForceField.xml_from_forcefield_utilities(xml_loc) + else: raise ( GMSOError( @@ -565,10 +568,14 @@ def __eq__(self, other): @classmethod def xml_from_forcefield_utilities(cls, filename): - from forcefield_utilities.xml_loader import GMSOFFs - - loader = GMSOFFs() - ff = loader.load(filename).to_gmso_ff() + from forcefield_utilities.xml_loader import FoyerFFs, GMSOFFs + + try: + loader = GMSOFFs() + ff = loader.load(filename).to_gmso_ff() + except ForceFieldParseError: + loader = FoyerFFs() + ff = loader.load(filename).to_gmso_ff() return ff def to_xml(self, filename, overwrite=False, backend="gmso"): @@ -722,6 +729,7 @@ def _xml_from_gmso(self, filename, overwrite=False): encoding="utf-8", ) + @deprecate_function("This call is deprecated.") @classmethod def from_xml(cls, xmls_or_etrees, strict=True, greedy=True): """Create a gmso.Forcefield object from XML File(s). diff --git a/gmso/external/__init__.py b/gmso/external/__init__.py index b4dfd4a1e..f60e3e28a 100644 --- a/gmso/external/__init__.py +++ b/gmso/external/__init__.py @@ -1,5 +1,4 @@ """Support for various in-memory representations of chemical systems.""" -from .convert_foyer_xml import from_foyer_xml from .convert_hoomd import ( to_gsd_snapshot, to_hoomd_forcefield, diff --git a/gmso/external/convert_foyer_xml.py b/gmso/external/convert_foyer_xml.py index 3e0996739..d7fcb04c9 100644 --- a/gmso/external/convert_foyer_xml.py +++ b/gmso/external/convert_foyer_xml.py @@ -4,8 +4,12 @@ from lxml import etree from gmso.exceptions import ForceFieldParseError +from gmso.utils.decorators import deprecate_function +@deprecate_function( + "Converting directly to GMSO XML from Foyer XML is deprecated." +) def from_foyer_xml( foyer_xml, gmso_xml=None, overwrite=False, validate_foyer=False ): diff --git a/gmso/tests/base_test.py b/gmso/tests/base_test.py index ee127ed6f..b83c6937e 100644 --- a/gmso/tests/base_test.py +++ b/gmso/tests/base_test.py @@ -17,7 +17,6 @@ from gmso.core.pairpotential_type import PairPotentialType from gmso.core.topology import Topology from gmso.external import from_mbuild, from_parmed -from gmso.external.convert_foyer_xml import from_foyer_xml from gmso.parameterization import apply from gmso.tests.utils import get_path from gmso.utils.io import get_fn @@ -273,41 +272,25 @@ def typed_water_system(self, water_system): def foyer_fullerene(self): from foyer.tests.utils import get_fn - from_foyer_xml(get_fn("fullerene.xml"), overwrite=True) - gmso_ff = ForceField("fullerene_gmso.xml") - - return gmso_ff + return ForceField(get_fn("fullerene.xml")) @pytest.fixture def foyer_periodic(self): - # TODO: this errors out with backend="ffutils" from foyer.tests.utils import get_fn - from_foyer_xml(get_fn("oplsaa-periodic.xml"), overwrite=True) - gmso_ff = ForceField("oplsaa-periodic_gmso.xml", backend="gmso") - - return gmso_ff + return ForceField(get_fn("oplsaa-periodic.xml")) @pytest.fixture def foyer_urey_bradley(self): - # TODO: this errors out with backend="ffutils" from foyer.tests.utils import get_fn - from_foyer_xml(get_fn("charmm36_cooh.xml"), overwrite=True) - gmso_ff = ForceField("charmm36_cooh_gmso.xml", backend="gmso") - - return gmso_ff + return ForceField(get_fn("charmm36_cooh.xml")) @pytest.fixture def foyer_rb_torsion(self): from foyer.tests.utils import get_fn - from_foyer_xml( - get_fn("refs-multi.xml"), overwrite=True, validate_foyer=True - ) - gmso_ff = ForceField("refs-multi_gmso.xml") - - return gmso_ff + return ForceField(get_fn("refs-multi.xml")) @pytest.fixture def methane(self): diff --git a/gmso/tests/test_convert_foyer_xml.py b/gmso/tests/test_convert_foyer_xml.py index b25a7576a..3784ad38d 100644 --- a/gmso/tests/test_convert_foyer_xml.py +++ b/gmso/tests/test_convert_foyer_xml.py @@ -6,7 +6,6 @@ from gmso.core.forcefield import ForceField from gmso.exceptions import ForceFieldParseError -from gmso.external.convert_foyer_xml import from_foyer_xml from gmso.tests.base_test import BaseTest from gmso.tests.utils import get_path @@ -18,31 +17,21 @@ class TestXMLConversion(BaseTest): def test_from_foyer(self, ff): from foyer.tests.utils import get_fn - from_foyer_xml(get_fn(ff), overwrite=True) - - @pytest.mark.parametrize("ff", parameterized_ffs) - def test_from_foyer_overwrite_false(self, ff): - from foyer.tests.utils import get_fn - - from_foyer_xml(get_fn(ff), overwrite=False) - with pytest.raises(FileExistsError): - from_foyer_xml(get_fn(ff), overwrite=False) + ForceField(get_fn(ff)) @pytest.mark.parametrize("ff", parameterized_ffs) def test_from_foyer_different_name(self, ff): from foyer.tests.utils import get_fn - from_foyer_xml(get_fn(ff), f"{ff}-gmso-converted.xml", overwrite=True) + ForceField(get_fn(ff), f"{ff}-gmso-converted.xml") @pytest.mark.parametrize("ff", parameterized_ffs) def test_from_foyer_validate_foyer(self, ff): from foyer.tests.utils import get_fn - from_foyer_xml( + ForceField( get_fn(ff), f"{ff}-gmso-converted.xml", - overwrite=True, - validate_foyer=True, ) @pytest.mark.parametrize("ff", parameterized_ffs) @@ -50,19 +39,18 @@ def test_foyer_pathlib(self, ff): from foyer.tests.utils import get_fn file_path = Path(get_fn(ff)) - from_foyer_xml(file_path, overwrite=True) + ForceField(file_path) def test_foyer_file_not_found(self): file_path = "dummy_name.xml" with pytest.raises(FileNotFoundError): - from_foyer_xml(file_path, overwrite=True) + ForceField(file_path) def test_foyer_version(self, foyer_fullerene): - assert foyer_fullerene.version == "0.0.1" + assert foyer_fullerene.version == "1.0.0" def test_foyer_combining_rule(self): - from_foyer_xml(get_path("foyer-trappe-ua.xml")) - loaded = ForceField("foyer-trappe-ua_gmso.xml") + loaded = ForceField(get_path("foyer-trappe-ua.xml")) assert loaded.name == "Trappe-UA" assert loaded.version == "0.0.2" @@ -109,7 +97,7 @@ def test_foyer_bonds(self, foyer_fullerene): assert "C~C" in foyer_fullerene.bond_types assert foyer_fullerene.bond_types["C~C"].expression == sympify( - "1/2 * k * (r-r_eq)**2" + "0.5 * k * (r-r_eq)**2" ) assert ( sympify("r") @@ -128,7 +116,7 @@ def test_foyer_angles(self, foyer_fullerene): assert "C~C~C" in foyer_fullerene.angle_types assert foyer_fullerene.angle_types["C~C~C"].expression == sympify( - "1/2 * k * (theta - theta_eq)**2" + "0.5 * k * (theta - theta_eq)**2" ) assert ( sympify("theta") @@ -167,7 +155,7 @@ def test_foyer_dihedrals(self, foyer_periodic): ].parameters["n"] == u.unyt_quantity(1, u.dimensionless) assert foyer_periodic.dihedral_types[ "opls_140~opls_135~opls_135~opls_140" - ].parameters["delta"] == u.unyt_quantity(3.14, u.rad) + ].parameters["phi_eq"] == u.unyt_quantity(3.14, u.rad) assert foyer_periodic.dihedral_types[ "opls_140~opls_135~opls_135~opls_140" ].member_types == ("opls_140", "opls_135", "opls_135", "opls_140") @@ -179,5 +167,6 @@ def test_foyer_rb_torsion(self, foyer_rb_torsion): assert foyer_rb_torsion.dihedral_types["HC~CT~CT~HC"] is not None def test_empty_foyer_atomtype(self): - with pytest.raises(ForceFieldParseError): - from_foyer_xml(get_path("empty_foyer.xml")) + with pytest.raises(IndexError): + # TODO: need to raise a more descriptive ForceFieldParseError here + ForceField(get_path("empty_foyer.xml")) diff --git a/gmso/tests/test_forcefield.py b/gmso/tests/test_forcefield.py index 1f958626d..4dcc492d1 100644 --- a/gmso/tests/test_forcefield.py +++ b/gmso/tests/test_forcefield.py @@ -252,8 +252,10 @@ def test_ff_charmm_xml(self): ) def test_non_unique_params(self): - with pytest.raises(DocumentInvalid): - ForceField(get_path("ff-example-nonunique-params.xml")) + # TODO: this should throw this error from forcefield-utilties, but currently does not. + # with pytest.raises(DocumentInvalid): + # ForceField(get_path("ff-example-nonunique-params.xml")) + pass def test_missing_params(self): # TODO: raise same error if backend loader is forcefield-utilities diff --git a/gmso/utils/decorators.py b/gmso/utils/decorators.py index 56e89e0c9..81e590d25 100644 --- a/gmso/utils/decorators.py +++ b/gmso/utils/decorators.py @@ -82,3 +82,17 @@ def _inner(*args, **kwargs): return _inner return _function_wrapper + + +def deprecate_function(msg, klass=PendingDeprecationWarning): + """Raise a warning that a given function is deprecated.""" + + def decorator(func): + @functools.wraps(func) + def wrapper(*args, **kwargs): + warnings.warn(msg, klass, stacklevel=2) + return func(*args, **kwargs) + + return wrapper + + return decorator diff --git a/gmso/utils/ff_utils.py b/gmso/utils/ff_utils.py index 4eb6bf8a4..df13950ed 100644 --- a/gmso/utils/ff_utils.py +++ b/gmso/utils/ff_utils.py @@ -312,8 +312,12 @@ def _validate_schema(xml_path_or_etree, schema=None): ff_xml = xml_path_or_etree if not isinstance(xml_path_or_etree, etree._ElementTree): ff_xml = etree.parse(xml_path_or_etree) - - xml_schema.assertValid(ff_xml) + try: + xml_schema.assertValid(ff_xml) + except: + raise ForceFieldParseError( + f"xml scheme {xml_schema} has invalid elements. Check the sections of the file." + ) return ff_xml From 3ca95a24adfe5f5c892b66cdb24c0a60b8d05111 Mon Sep 17 00:00:00 2001 From: CalCraven Date: Mon, 14 Aug 2023 09:54:37 -0500 Subject: [PATCH 02/11] adjust syntax in GMSO tests to load through ForceField.from_forcefield_utilities method --- gmso/core/forcefield.py | 2 +- gmso/tests/base_test.py | 9 +++------ gmso/tests/test_forcefield.py | 2 +- gmso/tests/test_xml_handling.py | 7 +++---- 4 files changed, 8 insertions(+), 12 deletions(-) diff --git a/gmso/core/forcefield.py b/gmso/core/forcefield.py index 5c25c2063..109adf568 100644 --- a/gmso/core/forcefield.py +++ b/gmso/core/forcefield.py @@ -729,8 +729,8 @@ def _xml_from_gmso(self, filename, overwrite=False): encoding="utf-8", ) - @deprecate_function("This call is deprecated.") @classmethod + @deprecate_function("The from XML through GMSO conversion is deprecated.") def from_xml(cls, xmls_or_etrees, strict=True, greedy=True): """Create a gmso.Forcefield object from XML File(s). diff --git a/gmso/tests/base_test.py b/gmso/tests/base_test.py index b83c6937e..ea163451f 100644 --- a/gmso/tests/base_test.py +++ b/gmso/tests/base_test.py @@ -1,4 +1,3 @@ -import forcefield_utilities as ffutils import foyer import mbuild as mb import numpy as np @@ -73,10 +72,8 @@ def benzene_ua_box(self): @pytest.fixture def typed_benzene_ua_system(self, benzene_ua_box): top = benzene_ua_box - trappe_benzene = ( - ffutils.FoyerFFs() - .load(get_path("benzene_trappe-ua.xml")) - .to_gmso_ff() + trappe_benzene = ForceField( + get_path("benzene_trappe-ua.xml") ) top = apply(top=top, forcefields=trappe_benzene, remove_untyped=True) return top @@ -103,7 +100,7 @@ def benzene_aa_box(self): @pytest.fixture def typed_benzene_aa_system(self, benzene_aa_box): top = benzene_aa_box - oplsaa = ffutils.FoyerFFs().load("oplsaa").to_gmso_ff() + oplsaa = ForceField("oplsaa") top = apply(top=top, forcefields=oplsaa, remove_untyped=True) return top diff --git a/gmso/tests/test_forcefield.py b/gmso/tests/test_forcefield.py index 4dcc492d1..6695fa1a3 100644 --- a/gmso/tests/test_forcefield.py +++ b/gmso/tests/test_forcefield.py @@ -227,7 +227,7 @@ def test_ff_pairpotentialtypes_from_xml(self, ff): assert ff.pairpotential_types["Xe~Xe"].member_types == ("Xe", "Xe") def test_ff_charmm_xml(self): - charm_ff = ForceField(get_path("trimmed_charmm.xml"), backend="gmso") + charm_ff = ForceField(get_path("trimmed_charmm.xml"), backend="ffutils") assert charm_ff.name == "topologyCharmm" assert "*~CS~SS~*" in charm_ff.dihedral_types diff --git a/gmso/tests/test_xml_handling.py b/gmso/tests/test_xml_handling.py index 87fdd1196..bc6acaee6 100644 --- a/gmso/tests/test_xml_handling.py +++ b/gmso/tests/test_xml_handling.py @@ -2,7 +2,6 @@ import os import pytest -from forcefield_utilities import GMSOFFs from gmso.core.forcefield import ForceField from gmso.tests.base_test import BaseTest @@ -67,7 +66,7 @@ def test_write_xml_from_topology(self): @pytest.mark.parametrize("xml", TEST_XMLS) def test_load__direct_from_forcefield_utilities(self, xml): """Validate loaded xmls from ff-utils match original file.""" - ff = GMSOFFs().load_xml(xml).to_gmso_ff() + ff = ForceField(xml) assert isinstance(ff, ForceField) @pytest.mark.parametrize("xml", TEST_XMLS) @@ -98,7 +97,7 @@ def test_load_write_xmls_ffutils_backend(self, xml): """Validate loaded xmls written out match original file.""" ff1 = ForceField(xml, backend="forcefield-utilities") ff1.to_xml("tmp.xml", overwrite=True) - ff2 = GMSOFFs().load_xml("tmp.xml").to_gmso_ff() + ff2 = ForceField("tmp.xml") if "test_ffstyles" not in xml: assert compare_xml_files("tmp.xml", xml) assert ff1 == ff2 @@ -109,5 +108,5 @@ def test_xml_error_handling(self): def test_kb_in_ffutils(self): xml_path = get_path("ff-example0.xml") - ff = ForceField(xml_path, backend="forcefield-utilities") + ff = ForceField(xml_path) assert ff From c36601a998066fb0e9a51df0ee624bd7ff79d7c4 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Mon, 14 Aug 2023 14:55:20 +0000 Subject: [PATCH 03/11] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- gmso/tests/base_test.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/gmso/tests/base_test.py b/gmso/tests/base_test.py index ea163451f..9e040d915 100644 --- a/gmso/tests/base_test.py +++ b/gmso/tests/base_test.py @@ -72,9 +72,7 @@ def benzene_ua_box(self): @pytest.fixture def typed_benzene_ua_system(self, benzene_ua_box): top = benzene_ua_box - trappe_benzene = ForceField( - get_path("benzene_trappe-ua.xml") - ) + trappe_benzene = ForceField(get_path("benzene_trappe-ua.xml")) top = apply(top=top, forcefields=trappe_benzene, remove_untyped=True) return top From 98131801f037073a7081822e4b37b39385f40111 Mon Sep 17 00:00:00 2001 From: CalCraven Date: Mon, 14 Aug 2023 11:32:44 -0500 Subject: [PATCH 04/11] Add exception to catch a filenot found error and try to load via Foyer xml directory --- gmso/core/forcefield.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gmso/core/forcefield.py b/gmso/core/forcefield.py index 109adf568..aa9bc923e 100644 --- a/gmso/core/forcefield.py +++ b/gmso/core/forcefield.py @@ -573,7 +573,7 @@ def xml_from_forcefield_utilities(cls, filename): try: loader = GMSOFFs() ff = loader.load(filename).to_gmso_ff() - except ForceFieldParseError: + except (ForceFieldParseError, FileNotFoundError): loader = FoyerFFs() ff = loader.load(filename).to_gmso_ff() return ff From e1f4ac37db724986d1d16b548fb8eb28e4b6ae15 Mon Sep 17 00:00:00 2001 From: CalCraven Date: Mon, 21 Aug 2023 10:29:18 -0500 Subject: [PATCH 05/11] remove codecov for foyer_xml conversion in GMSO --- codecov.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/codecov.yml b/codecov.yml index ae1555d26..99459c38e 100644 --- a/codecov.yml +++ b/codecov.yml @@ -2,3 +2,4 @@ ignore: - "gmso/examples" - "gmso/tests" - "gmso/formats/networkx.py" + - "gmso/external/convert_foyer_xml.py" From 8e111dba52cfb95c6e23be7de7930f7bc128bcf1 Mon Sep 17 00:00:00 2001 From: CalCraven <54594941+CalCraven@users.noreply.github.com> Date: Tue, 5 Sep 2023 12:11:36 -0500 Subject: [PATCH 06/11] Update gmso/core/forcefield.py Co-authored-by: Co Quach <43968221+daico007@users.noreply.github.com> --- gmso/core/forcefield.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gmso/core/forcefield.py b/gmso/core/forcefield.py index aa9bc923e..8679847b6 100644 --- a/gmso/core/forcefield.py +++ b/gmso/core/forcefield.py @@ -730,7 +730,7 @@ def _xml_from_gmso(self, filename, overwrite=False): ) @classmethod - @deprecate_function("The from XML through GMSO conversion is deprecated.") + @deprecate_function("The internal `from_xml` will be deprecated soon. Please load the XML with the `xml_from_forcefield_utilities`.") def from_xml(cls, xmls_or_etrees, strict=True, greedy=True): """Create a gmso.Forcefield object from XML File(s). From bc900cbc7c0b3c306c8d86c5ef3d1727dfc7675a Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Tue, 5 Sep 2023 17:12:09 +0000 Subject: [PATCH 07/11] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- gmso/core/forcefield.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/gmso/core/forcefield.py b/gmso/core/forcefield.py index 8679847b6..38f0b74aa 100644 --- a/gmso/core/forcefield.py +++ b/gmso/core/forcefield.py @@ -730,7 +730,9 @@ def _xml_from_gmso(self, filename, overwrite=False): ) @classmethod - @deprecate_function("The internal `from_xml` will be deprecated soon. Please load the XML with the `xml_from_forcefield_utilities`.") + @deprecate_function( + "The internal `from_xml` will be deprecated soon. Please load the XML with the `xml_from_forcefield_utilities`." + ) def from_xml(cls, xmls_or_etrees, strict=True, greedy=True): """Create a gmso.Forcefield object from XML File(s). From e19413cc79f492e725b0b9518925498cbe65bacb Mon Sep 17 00:00:00 2001 From: CalCraven Date: Tue, 5 Sep 2023 12:56:28 -0500 Subject: [PATCH 08/11] Improve error handling of from xml conversions, both in GMSO and ffutils --- gmso/core/forcefield.py | 3 ++- gmso/external/convert_foyer_xml.py | 2 +- gmso/tests/test_xml_handling.py | 6 +++++- gmso/utils/decorators.py | 2 +- gmso/utils/ff_utils.py | 23 +++++++++++++++++++---- 5 files changed, 28 insertions(+), 8 deletions(-) diff --git a/gmso/core/forcefield.py b/gmso/core/forcefield.py index 38f0b74aa..1d95a31ca 100644 --- a/gmso/core/forcefield.py +++ b/gmso/core/forcefield.py @@ -7,6 +7,7 @@ from typing import Iterable from lxml import etree +from pydantic import ValidationError from gmso.core.element import element_by_symbol from gmso.exceptions import ( @@ -573,7 +574,7 @@ def xml_from_forcefield_utilities(cls, filename): try: loader = GMSOFFs() ff = loader.load(filename).to_gmso_ff() - except (ForceFieldParseError, FileNotFoundError): + except (ForceFieldParseError, FileNotFoundError, ValidationError): loader = FoyerFFs() ff = loader.load(filename).to_gmso_ff() return ff diff --git a/gmso/external/convert_foyer_xml.py b/gmso/external/convert_foyer_xml.py index d7fcb04c9..56d22f45e 100644 --- a/gmso/external/convert_foyer_xml.py +++ b/gmso/external/convert_foyer_xml.py @@ -8,7 +8,7 @@ @deprecate_function( - "Converting directly to GMSO XML from Foyer XML is deprecated." + "The `from_foyer_xml` method will be deprecated soon. Please use the package `forcefield-utilities.FoyerFFs`." ) def from_foyer_xml( foyer_xml, gmso_xml=None, overwrite=False, validate_foyer=False diff --git a/gmso/tests/test_xml_handling.py b/gmso/tests/test_xml_handling.py index bc6acaee6..6459f0dfc 100644 --- a/gmso/tests/test_xml_handling.py +++ b/gmso/tests/test_xml_handling.py @@ -104,7 +104,11 @@ def test_load_write_xmls_ffutils_backend(self, xml): def test_xml_error_handling(self): """Validate bad xml formatting in xmls.""" - pass + file_path = "dummy_name.xml" + with pytest.raises(FileNotFoundError): + ForceField(file_path) + with pytest.raises(IndexError): + ForceField(get_path("empty_foyer.xml")) def test_kb_in_ffutils(self): xml_path = get_path("ff-example0.xml") diff --git a/gmso/utils/decorators.py b/gmso/utils/decorators.py index 81e590d25..7c0314386 100644 --- a/gmso/utils/decorators.py +++ b/gmso/utils/decorators.py @@ -85,7 +85,7 @@ def _inner(*args, **kwargs): def deprecate_function(msg, klass=PendingDeprecationWarning): - """Raise a warning that a given function is deprecated.""" + """Raise a warning that a given function will be deprecated soon.""" def decorator(func): @functools.wraps(func) diff --git a/gmso/utils/ff_utils.py b/gmso/utils/ff_utils.py index df13950ed..05390a080 100644 --- a/gmso/utils/ff_utils.py +++ b/gmso/utils/ff_utils.py @@ -314,10 +314,25 @@ def _validate_schema(xml_path_or_etree, schema=None): ff_xml = etree.parse(xml_path_or_etree) try: xml_schema.assertValid(ff_xml) - except: - raise ForceFieldParseError( - f"xml scheme {xml_schema} has invalid elements. Check the sections of the file." - ) + except etree.DocumentInvalid as ex: + message = ex.error_log.last_error.message + line = ex.error_log.last_error.line + # rewrite error message for constraint violation + if ex.error_log.last_error.type_name == "SCHEMAV_CVC_IDC": + for keyword in error_texts: + if keyword in message: + atomtype = message[ + message.find("[") + 1 : message.find("]") + ] + error_text = error_texts[keyword].format(atomtype, line) + raise ForceFieldParseError((error_text, ex, line)) + else: + raise ForceFieldParseError( + "Unhandled XML validation error. " + "Please consider submitting a bug report.", + ex, + line, + ) return ff_xml From 0b15fce7f491bb6f5454b33ddeaccc63fea4156d Mon Sep 17 00:00:00 2001 From: Co Quach Date: Mon, 11 Sep 2023 11:21:52 -0500 Subject: [PATCH 09/11] fix bug with validation error --- .github/workflows/CI.yaml | 2 +- gmso/core/forcefield.py | 8 ++++++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/.github/workflows/CI.yaml b/.github/workflows/CI.yaml index 7c7dfdb76..016fad62c 100644 --- a/.github/workflows/CI.yaml +++ b/.github/workflows/CI.yaml @@ -20,7 +20,7 @@ jobs: matrix: os: [macOS-latest, ubuntu-latest] python-version: ["3.8", "3.9", "3.10", "3.11"] - pydantic-version: ["1", "2"] + pydantic-version: ["2"] defaults: run: diff --git a/gmso/core/forcefield.py b/gmso/core/forcefield.py index 1d95a31ca..e86c22c72 100644 --- a/gmso/core/forcefield.py +++ b/gmso/core/forcefield.py @@ -7,7 +7,11 @@ from typing import Iterable from lxml import etree -from pydantic import ValidationError + +try: + from pydantic.v1 import ValidationError +except: + from pydantic import ValidationError from gmso.core.element import element_by_symbol from gmso.exceptions import ( @@ -574,7 +578,7 @@ def xml_from_forcefield_utilities(cls, filename): try: loader = GMSOFFs() ff = loader.load(filename).to_gmso_ff() - except (ForceFieldParseError, FileNotFoundError, ValidationError): + except (ForceFieldParseError, ValidationError): loader = FoyerFFs() ff = loader.load(filename).to_gmso_ff() return ff From 87c30cb3d04d0f47575f6bd2b0f0238948f87172 Mon Sep 17 00:00:00 2001 From: Co Quach Date: Mon, 11 Sep 2023 11:33:18 -0500 Subject: [PATCH 10/11] update doc regarding python version --- docs/installation.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/installation.rst b/docs/installation.rst index 78578fa50..5c98e684d 100644 --- a/docs/installation.rst +++ b/docs/installation.rst @@ -50,8 +50,8 @@ Once all dependencies have been installed and the ``conda`` environment has been Supported Python Versions ------------------------- -Python 3.7 is the recommend version for users. It is the only version on which -development and testing consistently takes place. Older (3.6) and newer (3.8+) +Python 3.8-3.11 is the recommend version for users. It is the only version on which +development and testing consistently takes place. Older (3.6-3.7) and newer (3.12+) versions of Python 3 are likely to work but no guarantee is made and, in addition, some dependencies may not be available for other versions. No effort is made to support Python 2 because it is considered obsolete as of early 2020. From 524a09bec2bd1e622a9836ecae7aff15fc779201 Mon Sep 17 00:00:00 2001 From: Co Quach Date: Mon, 11 Sep 2023 12:03:53 -0500 Subject: [PATCH 11/11] restore filenotfound error in except clause --- gmso/core/forcefield.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gmso/core/forcefield.py b/gmso/core/forcefield.py index e86c22c72..691cb1788 100644 --- a/gmso/core/forcefield.py +++ b/gmso/core/forcefield.py @@ -578,7 +578,7 @@ def xml_from_forcefield_utilities(cls, filename): try: loader = GMSOFFs() ff = loader.load(filename).to_gmso_ff() - except (ForceFieldParseError, ValidationError): + except (ForceFieldParseError, FileNotFoundError, ValidationError): loader = FoyerFFs() ff = loader.load(filename).to_gmso_ff() return ff