mosdef-hub / gmso

Flexible storage of chemical topology for molecular simulation
https://gmso.mosdef.org
MIT License
52 stars 33 forks source link

Add Ruff to pre-commit #819

Closed CalCraven closed 5 months ago

CalCraven commented 5 months ago

This is just a test PR to test the linting with Ruff, which is recommended by pydocstyle as a replacement and used by other big python package.

CalCraven commented 5 months ago

Looks like the pre-commit has a lot of recommendations for changes. @daico007 I think we should look at some custom parsing, because notes like the __init__.py shouldn't be following the same parsing guidelines.

chrisjonesBSU commented 5 months ago

If the only issue is the F401 errors in the __init__.py files then maybe we can just ignore that specific error in those files.

See this in the documentation

CalCraven commented 5 months ago

If the only issue is the F401 errors in the init.py files then maybe we can just ignore that specific error in those files.

Yes that would be good. We may have to do that with a few of the different files this way because it's a bunch of files across the project.

chrisjonesBSU commented 5 months ago

Reading the ruff documentation https://docs.astral.sh/ruff/linter/#error-suppression

We can add # ruff: noqa: F401 to the top of any __init__.py files which may be easier than setting the paths to ignore in a config file.

chrisjonesBSU commented 5 months ago

The rest of the suggested changes seem pretty reasonable I think. What do you all think @CalCraven @daico007 ?

CalCraven commented 5 months ago

I would say we should ignore things for the tests module. Otherwise, I think it would be wise to perform the rest of the changes just to improper our PEP compliance.

chrisjonesBSU commented 5 months ago

I think I got most of the errors fixed (even in the tests). There are some lingering ones about not using lambda and not using bare excepts.

I think we should fix the bare excepts, I just don't know what the errors should be off the top of my head. We use lambda functions quite a bit, so maybe we just ignore that specific error in the config file?

CalCraven commented 5 months ago

https://github.com/BrianPugh/python-template/blob/main/.pre-commit-config.yaml

Going to link to a cookie-cutter version of a ruff linter, check out the pyproject.toml as well for some additional formatting stuff we could consider implementing, such as type checking and isort.