Modify XML parsing order of general forcefield #488
Conversation
Add validation step to perform XML conversion only if foyer XML is provided
|
This pull request introduces 2 alerts when merging d2556f8 into 60a738d - view on LGTM.com new alerts:
|
|
This pull request introduces 2 alerts when merging 4614ac1 into ab1d755 - view on LGTM.com new alerts:
|
for more information, see https://pre-commit.ci
Codecov Report
@@ Coverage Diff @@
## master #488 +/- ##
=======================================
Coverage 73.63% 73.63%
=======================================
Files 17 17
Lines 1866 1866
=======================================
Hits 1374 1374
Misses 492 492 |
|
This pull request introduces 2 alerts when merging d1a96c0 into ab1d755 - view on LGTM.com new alerts:
|
…rder_xml_parsing_sequence
…co007/foyer into reorder_xml_parsing_sequence
|
This pull request introduces 2 alerts when merging e35e444 into ab1d755 - view on LGTM.com new alerts:
|
|
This pull request introduces 2 alerts when merging ec171c8 into ab1d755 - view on LGTM.com new alerts:
|
…rder_xml_parsing_sequence
|
This pull request introduces 2 alerts when merging 8ee6f5f into c5802ba - view on LGTM.com new alerts:
|
| from_foyer_xml( | ||
| foyer_xml=str(file), | ||
| gmso_xml=str(tempfile.name), | ||
| overwrite=True, | ||
| ) | ||
| tmp_processed_files.append(tempfile.name) | ||
| except Exception: | ||
| except: |
There was a problem hiding this comment.
Should this be a ValidationError? That's what it seems like the Validator is setup to raise, see line in validator.py. Might be better to not raise such a general exception, unless there are other errors that might arise from the validation.
There was a problem hiding this comment.
The current foyer validator is more to check if an atom type is provided provide a malformed SMARTS, or is missing a nonbonded, etc. Trying to validating a GMSO XML will raise error, but not ValidationError, more like XML parsing error (This element is not expected). So I think having a general catch here is ok, since if there is any other issues, it would be raised by the GMSO validator.
…rder_xml_parsing_sequence
|
This pull request introduces 2 alerts when merging b1f0271 into 11c144c - view on LGTM.com new alerts:
|
|
This pull request introduces 2 alerts when merging d671664 into 11c144c - view on LGTM.com new alerts:
|
PR Summary:
Add validation step to perform XML conversion only if foyer XML is provided (else load directly by gmso).
PR Checklist