deprecate any foyer to gmso conversion that does not use forcefield u…

…tilities. Updates tests to load ff from XML through ff-utils

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

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

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).

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,

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
 ):

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):

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,51 +17,40 @@ 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)
     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"))

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

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

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