Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SUGGESTION] Adding pre-commit hooks for linting/formatting the code #10

Closed
ezpzbz opened this issue May 16, 2021 · 1 comment
Closed

Comments

@ezpzbz
Copy link

ezpzbz commented May 16, 2021

Hi,
Thanks for developing this nice package.

While I was reading/reviwing the code, I found couple of unused imports/variables (listed below). I suggest adding pre-commit hooks with pylint, yapf, and prospector to format/lint code at commits. This would capture these and also improves readability of the code.

Unused imports:

  • os,mpl,plt, Element, Lattice, and SiteCollection in analysis.py.
  • Structure and numpy in convergence.py.
  • os in generation.py.
  • mpl in io.py.
  • Slab, Element, PeriodicSite in vasp_data.py.
  • warnings and Structure in test_analysis.py.
  • warnings, os, Structure, and Slab in test_convergence.py
  • warnings, Structure in test_generation.py
  • warnings, os, Structure, surfaxe.io in test_io.py

Unused variables:

Best regards,
Pezhman

@brlec
Copy link
Collaborator

brlec commented May 19, 2021

Hi,

Thank you for reviewing the code and flagging these! I have now removed the unused imports in the latest commit on develop - these will be merged with master once the JOSS review is complete. Regarding unused variables, pylint doesn't pick up on where low and high are referenced later on in dataframe query, so they were left in.

Best,
Kat

@brlec brlec closed this as completed May 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants