Conversation
|
@JanosJiri, budes mit cas. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #208 +/- ##
=======================================
Coverage 84.05% 84.05%
=======================================
Files 47 47
Lines 6599 6599
Branches 764 764
=======================================
Hits 5547 5547
Misses 853 853
Partials 199 199
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
JanosJiri
left a comment
JanosJiri
left a comment
There was a problem hiding this comment.
Great work bringing a new interface to ABIN. I have some comments to address though as I didn't get some things.
danielhollas
left a comment
danielhollas
left a comment
There was a problem hiding this comment.
This is awesome to see, impressive and non-trivial work! 👏
Just one suggestion with regards to catching errors.
Overall I think here you've almost reached the limit of practicality of bash scripts.
At this point I'd consider a more structured python script as an interface (but we currently don't have any python interfaces to start from, and Python has non-trivial startup cost (10-20ms), but that probably would not matter in this case.
(not saying anything should be changed here, just thinking out loud).
JanosJiri
left a comment
JanosJiri
left a comment
There was a problem hiding this comment.
Awesome, thanks for the modifications. I've left a single comment and I see @danielhollas had two comments too. But otherwise I am happy with that.
|
Thanks for all the suggestions. Everything should be resolved now. |
danielhollas
left a comment
danielhollas
left a comment
There was a problem hiding this comment.
Amazing work, thank you!
3 participants
Adding ChemShell interface for QM/MM calculation. Both standard and LZ version.