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/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" 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. diff --git a/gmso/core/forcefield.py b/gmso/core/forcefield.py index e7f86699b..691cb1788 100644 --- a/gmso/core/forcefield.py +++ b/gmso/core/forcefield.py @@ -8,10 +8,19 @@ from lxml import etree +try: + from pydantic.v1 import ValidationError +except: + from pydantic import ValidationError + 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 +54,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 +111,7 @@ def __init__( "ffutils", ]: ff = ForceField.xml_from_forcefield_utilities(xml_loc) + else: raise ( GMSOError( @@ -565,10 +573,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, FileNotFoundError, ValidationError): + loader = FoyerFFs() + ff = loader.load(filename).to_gmso_ff() return ff def to_xml(self, filename, overwrite=False, backend="gmso"): @@ -723,6 +735,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`." + ) 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..56d22f45e 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( + "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/base_test.py b/gmso/tests/base_test.py index ee127ed6f..9e040d915 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 @@ -17,7 +16,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 @@ -74,11 +72,7 @@ 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 @@ -104,7 +98,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 @@ -273,41 +267,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..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 @@ -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/tests/test_xml_handling.py b/gmso/tests/test_xml_handling.py index 87fdd1196..6459f0dfc 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,16 +97,20 @@ 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 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") - ff = ForceField(xml_path, backend="forcefield-utilities") + ff = ForceField(xml_path) assert ff diff --git a/gmso/utils/decorators.py b/gmso/utils/decorators.py index 56e89e0c9..7c0314386 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 will be deprecated soon.""" + + 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..05390a080 100644 --- a/gmso/utils/ff_utils.py +++ b/gmso/utils/ff_utils.py @@ -312,8 +312,27 @@ 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 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