Closed umesh-timalsina closed 3 years ago
This PR is now complete for initial round of reviews.
This pull request introduces 2 alerts when merging f969c13a4ced0e50977197efff8c2a3b3bce0b0d into e80015c98324b3eab77457fb35df98c2dcd39cd8 - view on LGTM.com
new alerts:
Merging #389 (b0b120b) into master (8953210) will increase coverage by
0.34%
. The diff coverage is95.87%
.:exclamation: Current head b0b120b differs from pull request most recent head bd2c296. Consider uploading reports for the commit bd2c296 to get more accurate results
@@ Coverage Diff @@
## master #389 +/- ##
==========================================
+ Coverage 85.08% 85.42% +0.34%
==========================================
Files 15 16 +1
Lines 1448 1496 +48
==========================================
+ Hits 1232 1278 +46
- Misses 216 218 +2
This issue has been discussed at length in #386. Specifically this PR is intended to address https://github.com/mosdef-hub/foyer/issues/386#issuecomment-794684790 and with #382 underway. I think we can safely commit to removing third party packages for atomtyping engine from foyer.
Checklist:
atom_typer.py
smarts_graph.py
Based on the third point a utility class could look like the following: