Merge pull request #4956 from epretti/ff-include

Handle relative include paths from force fields in data directories

wrappers/python/openmm/app/forcefield.py

@@ -245,29 +245,24 @@ class ForceField(object):
         i = 0
         while i < len(files):
             file = files[i]
-            tree = None
-            try:
-                # this handles either filenames or open file-like objects
-                tree = etree.parse(file)
-            except IOError:
+            # this handles either filenames or open file-like objects
+            if isinstance(file, str) and not os.path.isfile(file):
                 for dataDir in _getDataDirectories():
                     f = os.path.join(dataDir, file)
                     if os.path.isfile(f):
-                        tree = etree.parse(f)
+                        file = f
                         break
+            try:
+                tree = etree.parse(file)
+            except FileNotFoundError:
+                raise ValueError('Could not locate file "%s"' % file)
             except Exception as e:
                 # Fail with an error message about which file could not be read.
-                # TODO: Also handle case where fallback to 'data' directory encounters problems,
-                # but this is much less worrisome because we control those files.
-                msg  = str(e) + '\n'
                 if hasattr(file, 'name'):
                     filename = file.name
                 else:
                     filename = str(file)
-                msg += "ForceField.loadFile() encountered an error reading file '%s'\n" % filename
-                raise Exception(msg)
-            if tree is None:
-                raise ValueError('Could not locate file "%s"' % file)
+                raise Exception('ForceField.loadFile() encountered an error reading file "%s": %s' % (filename, e))
 
             trees.append(tree)
             i += 1

wrappers/python/tests/TestForceField.py

@@ -6,6 +6,8 @@ from openmm.unit import *
 import openmm.app.element as elem
 import openmm.app.forcefield as forcefield
 import math
+import shutil
+import tempfile
 import textwrap
 try:
     from cStringIO import StringIO
@@ -1254,6 +1256,23 @@ class TestForceField(unittest.TestCase):
         self.assertTrue('spce-O' in forcefield._atomTypes)
         self.assertTrue('HOH' in forcefield._templates)
 
+    def test_IncludesFromDataDirectory(self):
+        """Test relative include paths from subdirectories of the data directory."""
+
+        oldDataDirs = forcefield._dataDirectories
+        try:
+            with tempfile.TemporaryDirectory() as tempDataDir:
+                forcefield._dataDirectories = forcefield._getDataDirectories() + [tempDataDir]
+                os.mkdir(os.path.join(tempDataDir, 'subdir'))
+                for testFileName in ['ff_with_includes.xml', 'test_amber_ff.xml']:
+                    shutil.copyfile(os.path.join('systems', testFileName), os.path.join(tempDataDir, 'subdir', testFileName))
+                ff = ForceField(os.path.join('subdir', 'ff_with_includes.xml'))
+                self.assertTrue(len(ff._atomTypes) > 10)
+                self.assertTrue('spce-O' in ff._atomTypes)
+                self.assertTrue('HOH' in ff._templates)
+        finally:
+            forcefield._dataDirectories = oldDataDirs
+
     def test_ImpropersOrdering(self):
         """Test correctness of the ordering of atom indexes in improper torsions
         and the torsion.ordering parameter.