🔧 MAINTAIN: Add isort pre-commit hook #5151
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #5151 +/- ##
===========================================
- Coverage 80.92% 80.89% -0.02%
===========================================
Files 536 536
Lines 37080 37019 -61
===========================================
- Hits 30004 29944 -60
+ Misses 7076 7075 -1
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
pyproject.toml
Outdated
| @@ -75,6 +75,13 @@ markers = [ | |||
| "sphinx: set parameters for the sphinx `app` fixture" | |||
| ] | |||
|
|
|||
| [tool.isort] | |||
| line_length = 100 | |||
There was a problem hiding this comment.
Why not 120 as the rest of the code?
.pre-commit-config.yaml
Outdated
| (?x)^( | ||
| aiida/common/.*py| | ||
| )$ |
There was a problem hiding this comment.
I can appreciate that doing everything now would cause merge conflicts with open PRs, but I fear that in this way it is never going to get done. If we are going to add this, I prefer we bite the bullet. Since it only concerns imports, the merge conflicts should not be complicated.
There was a problem hiding this comment.
Sure ting, more lines of code attributed to me lol
|
right, removed the restriction on files; lets see if pre-commit.ci does its thing! |
for more information, see https://pre-commit.ci
| def upgrade_schema_version(up_revision, down_revision): | ||
| from functools import partial |
There was a problem hiding this comment.
weird, only because of the extra new line, does pylint start failing it for not having a docstring 🤷 (https://github.com/aiidateam/aiida-core/runs/3690148576)
| def upgrade_schema_version(up_revision, down_revision): | |
| from functools import partial | |
| def upgrade_schema_version(up_revision, down_revision): | |
| """Run migrations, to translate the database schema.""" | |
| from functools import partial |
|
Ok @sphuber I think this is good to go |
To nicely sort imports: https://pycqa.github.io/isort/
I've set it as "opt-in" (via file regex) for now, so we can introduce it gradually, without introducing too many merge conflicts in existing PRsAnnoyingly, there is no specific integration with yapf (as there is for black), but from googling the configuration I added should be fine.
I added
force_sort_within_sections=trueto not splitimport aandfrom a importtypes, which I feel is nicer