Codecov Report

Base: 76.33% // Head: 76.92% // Increases project coverage by +0.59% 🎉

Coverage data is based on head (8c11e3b) compared to base (9309ba8).
Patch has no changes to coverable lines.

@@            Coverage Diff             @@
##             main     #242      +/-   ##
==========================================
+ Coverage   76.33%   76.92%   +0.59%     
==========================================
  Files           4        4              
  Lines         845      845              
==========================================
+ Hits          645      650       +5     
+ Misses        200      195       -5     

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@mattwthompson : Thanks so much for doing this!

It looks like test_ffxml_import is showing amber/opc3_standard.xml is now failing with

E               ValueError: Could not locate file "ions/ionslm_126_opc3.xml"

I believe this is because the newly added ffxml/amber/opc3_standard.xml omits the amber/ at the front of the path:

 <Include file="ions/ionslm_126_opc3.xml"/>

The same happens for the newly added ffxml/amber/opc_standard.xml.

Are you able to prepend amber/ in those paths, or should @peastman try to debug?

I have pytest openmmforcefields/tests/test_amber_import.py::test_ffxml_import -nauto passing locally - we'll see if the runners agree

Okay, CI is passing now; there were some non-trivial changes brought in by merging upstream, so it might be worth another look. I'm not sure if the amber/ folder should be blown away or not, the two XML files could be duplicates or unused.

I also can't comment on the meat of the original PR #239 - my objective here is to get that work closer to the finish line; feel free to push directly to this branch if more work is needed.

Would be good to have @aizvorski take a look at this

@mattwthompson Thank you very much for doing this! That was a tough merge

A couple of renames and this should be good to go:

amber/opc.xml -> openmmforcefields/ffxml/amber/opc.xml
amber/opc3.xml -> openmmforcefields/ffxml/amber/opc3.xml
(unlike other xml files in amber/ dir, these are meant to be part of the final output not just feedstocks)

Could you possibly do that? I'd do it but I'm not sure how :)

Let's see if that works

Okay, I think that worked. @aizvorski good to merge, do you think? Or @jchodera can merge when he gets to this next. I just don't want to do it without a final check.

@mattwthompson Yep, good to merge! Thanks again