mdanalysis
mdanalysis copied to clipboard
Issue 3449 lammps dump parser
Fixes #
Changes made in this Pull Request:
- LAMMPS standard atom styles are added to
DATAParser, so it can now handle all LAMMPS atom styles as listed in read_data and atom_style commands. Consequently, the former freedom in passing a user-define atom style viaatom_stylekwarg is replaced with replaced with a list of pre-defined atom styles. Accordingly, all the relevant parsing methods are changed. - The
headerdictionary is passed as a new argument to all parsing method for satiny check. Now, each parsing such as_parse_massesor_parse_poscheck whether the length ofdatalinesis equal to a corresponding item inheaderdictionary or not. - All the field(s) words in the documentations or comments changed to attribute(s) since the later is the correct physical/technical term used in LAMMPS or MDAnalysis for referring to a physical attributes of an atom.
- Some cosmetic changes done; for instance, unused imports deleted.
Note
- This pull request does not address #3449 directly, but it generalizes the process of parsing LAMMPS topology information from DATA an/or DUMP files.
- A pull for modifying
LammpsDumpParseris request shortly after this one.
PR Checklist
- [ ] Tests?
- [x] Docs?
- [ ] CHANGELOG updated?
- [ ] Issue raised/referenced?
Hello @amirhs1! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:
There are currently no PEP 8 issues detected in this Pull Request. Cheers! :beers:
Comment last updated at 2022-06-14 13:46:20 UTC
Hi @amirhs1 you seem to have added a bunch of files to the diff (and removed some) that you did not intend to (29 files changed). Please fix this up so only the right files appear in the diff before we go on to review your PR :)
Hi Hugo!
I did not delete or change any other file personally, but let me check. Is it possible that a cloud service such iCloud added some hidden files to the repo directory on my machine? And then git commit those files?
Amir
On Apr 18, 2022, at 06:05, Hugo MacDermott-Opeskin @.***> wrote:
Hi @amirhs1 you seem to have added a bunch of files to the diff (and removed some) that you did not intend to (29 files changed). Please fix this up so only the right files appear in the diff before we go on to review your PR :)
— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.
Did you use git commit * at any point ? That can sometimes do it. Have a look in the Files Changed tab in this PR.
Did you use
git commit *at any point ? That can sometimes do it. Have a look in theFiles Changedtab in this PR.
No! Let me check.
@hmacdope is it ok now?
@amirhs1 yep the diff looks good now, I'll review as soon as I can.
In my pull request, I limited the functionality of the atom_style kwarg, so it only accepts LAMMPS standard data formats. However, this limitation can be lifted and the atom_style can also accept user-defined styles. What do you think?
In the case of LAMMPSDumpParser, I did not add an atom_style kwarg; instead, the code itself infers the column names for the information provided in the LAMMPSDUMP file. This also can be changed, so a atom_style kwarg can be defined that accepts both the user-defined styles and LAMMPS standard dump styles. Again, what do you think?
Sorry for my long absence! I was on a vacation for 5 weeks!
@amirhs1 very jealous. Ping me if you need review.