-
Notifications
You must be signed in to change notification settings - Fork 26
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
Implement black and isort-based style checks #549
Conversation
66aec4e
to
9d41139
Compare
This is ready for review. For reviewers: there are two commits. The first commit contains the manual changes; the second is entirely auto-generated using |
For the record, I don't have strong feelings on the line length change here - if the consensus is that people would prefer not to change this, I'll revert and regenerate the changes. |
@@ -1,4 +1,5 @@ | |||
[flake8] | |||
max-line-length = 79 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks like the max is still 79?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I changed it back following discussion in other channels.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can isort do sorting with lowercase after captials?
One comment about line lengths in flake8.
Not going to argue on the line-length thing - I'm tempted to say just take whatever default black has and that way it'll track whatever "community standards" are going forward.
Nothing else looks out-of-place after a quick skim of the diff
It can - there's an |
This will interfere with other open PRs, so I plan to merge the other PRs first (not including the changelog) PR and then redo the automated style fixes if necessary. |
e4db865
to
8dbce11
Compare
I did an evil force push to bring this up to date with However, I ran into a snag: the current version of black will reformat some existing line-length-compliant docstrings in such a way that they introduce new line length violations, so I had to make manual fix-ups to those docstrings to get them into a state where (a) they passed the line length check, and (b) black won't reformat them to something that no longer passes the line length check. Upstream PR that introduced this issue: psf/black#3430 |
This PR adds
black
andisort
style formatting and checking, and requires future code changes to comply withblack
andisort
styles.check-style.yml
that runsblack
,isort
andflake8
checksflake8
check from test workflowetstool.py
support for both checking and fixing style, via newstyle
command (with subcommandsfix
andcheck
)flake8
command frometstool.py
Note: I'm installing the style-related packages from PyPI in
etstool.py
, so thatetstool.py
and the new workflow are installing the same package versions. I'm also fixing black to version 23.x so that we don't suddenly get style failures for the first Black release in 2024 (or later years) - see Black's stability policy here: https://black.readthedocs.io/en/stable/the_black_code_style/index.html#stability-policyTo do:
Closes #527