adjusted xyz_to_mol to take in different forms of xyz data#5
Conversation
jwaldrop107
left a comment
jwaldrop107
left a comment
There was a problem hiding this comment.
I think the changes here are mechanically fine, but I would actually advise introducing a new module entirely. Basically, split what you have here into one module whose singular focus is to parse XYZ data from a string, and another one whose purpose is to read in XYZ data from a file. The file reader module can then use the data parser as a submodule. This scheme allows for clarity of purpose in the modules and distinct names to describe what they do ("XYZ To Molecule" and "XYZ File To Molecule" for instance).
Regarding the completeness of the test cases, they seem sufficient to me.
|
Agree with @jwaldrop107. |
|
I separated out the modules, the only thing I'm not super confident about is the "try, catch" code. |
jwaldrop107
left a comment
jwaldrop107
left a comment
There was a problem hiding this comment.
Right track, but there are some further changes I'd recommend.
|
🚀 [bumpr] Bumped! |
3 participants
Is this pull request associated with an issue(s)?
No
Description
I wanted to adjust the xyz_to_mol module to be able to take in data from the input as a string instead of just as a file.
TODOs