Add version attribute to force fields

[mattwthompson]

PR Summary:

Fixes #187, would be useful in #290

• Adds version as an attribute of the ForceField element in the XML schema
• Adds version as an attribute to the Forcefield class
• Adds a simple test via reading a toy XML file and asserting the above two versions are equal

This is not done elegantly right now; 99% of the XML loading is done by OpenMM, so I had to do this after the fact and only dealing with the case of passing a single XML, which is all I believe we do right now. I'm not sure what the target for this PR should be, but I think we should soon have a bigger discussion about how we want to handle multiple XML files. The OpenMM use case is basically Forcefield('protein_model.xml', 'water_model.xml'), i.e. there is a clean interface between constituents and not any expected overlap between the two. This doesn't cleanly map on to our use cases, I don't think. Related to #297 #262 #149

If this seems reasonable I can do the same for names (#153), here or in another PR. Requesting a few specific reviewers since I have discussed this with many people over the past year or two (but only gotten to it now 🤕)

PR Checklist

---

• Includes appropriate unit test(s)
• Appropriate docstring(s) are added/updated
• Code is (approximately) PEP8 compliant
• Issue(s) raised/addressed?

[ahy3nz]

 /Users/ayang41/Programs/foyer/foyer/tests/test_forcefield.py(511)test_load_version_number()
-> assert lj_ff.version == '0.4.1'
(Pdb) l
506  	            assert str(item.xpath('comment()')) == '[<!--Note: original overrides="opls_144"-->]'
507
508  	def test_load_version_number():
509  	    lj_ff = Forcefield(get_fn('lj.xml'))
510  	    import pdb; pdb.set_trace()
511  ->	    assert lj_ff.version == '0.4.1'
[EOF]
(Pdb) foo = Forcefield(forcefield_files=[get_fn('lj.xml')])
(Pdb) foo.version
'foobar'
(Pdb) lj_ff.version
'0.4.1'
(Pdb)

• You have an extra foobar debug line in the init
• Did you want to cover the case where someone passes a list of XML files? This PR appears to only cover the case of passing a single XML string, not a list (which is what most people tend to do anyways...)

[mattwthompson]

Did you want to cover the case where someone passes a list of XML files? This PR appears to only cover the case of passing a single XML string, not a list (which is what most people tend to do anyways...)

I should be able to rework it to handle multiple files but I also think this might be a feature we want to drop at some point.

[codecov]

Codecov Report

Merging #298 into master will increase coverage by 0.32%.
The diff coverage is 100%.

@@            Coverage Diff            @@
##           master    #298      +/-   ##
=========================================
+ Coverage   86.57%   86.9%   +0.32%     
=========================================
  Files          14      14              
  Lines        1222    1252      +30     
=========================================
+ Hits         1058    1088      +30     
  Misses        164     164

[mattwthompson]

I'm happy with where this is at now. If this approach is ok, I will duplicate this functionality to support a name attribute.

[mikemhenry]

Can you add a warning if there is no version? That way people can see that they should add a version if it is missing

[justinGilmer]

Aside from adding in a warning about the version or name, looks good to me!

[justinGilmer]

Going off of Mike's comment about raising a warning, this seems like the place where that would go.

Suggested change

	except KeyError:
	except KeyError:
	warn.Warn("version warning message here")
	return None

[uppittu11]

This looks good to me.

Another thing we could add is an attribute to the Forcefield class which gives the path from which the forcefield was loaded. This wouldn't be saved in the forcefield xml, but is only an attribute. This might be a dumb idea, since we might have issues comparing two of the same forcefield loaded from different paths.

Request implemented