-
Notifications
You must be signed in to change notification settings - Fork 525
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
Make the entire code base PEP8 compliant. #329
Comments
IDAES recently implemented the usage of |
I have one more big PR coming that it would be nice to wait a bit and not conflict. I am still working to get a couple tests passing and then can put up a WIP PR so that you can (hopefully) avoid those files while you start applying black. Should we do black in chunks to keep the PR process simpler, or do it in one big chunk so that we only need to have a small number of revs in the |
I intend to do the Also, for posterity, I intend to run this command: |
Double check these files to make sure the line numbers are correct after applying black The problematic line was replaced in #2754 |
Will black also affect contrib directories? |
@bernalde yes but we're applying it in phases to reduce merge conflicts as much as possible with currently open PRs (including the MindtPy rewrite). If you have a specific module in contrib that you want us to wait to apply it to let us know. |
None in particular right now. I would say that for those of us writing code, it might be better if we are told how to implement the formatting correctly. Something along the lines of https://jump.dev/JuMP.jl/stable/developers/style/#Style-guide |
Yep, we will definitely document formatting expectations after finishing this first pass. I expect we'll also need some automated infrastructure to make sure new PRs are complying but we haven't discussed what this will look like yet. |
I have a plan to implement something similar to what IDAES does in that it has a linting check at the beginning of the job pipeline - if the linting is wrong, none of the other jobs will run. |
Adding another comment for posterity:
|
For closure:
|
No description provided.
The text was updated successfully, but these errors were encountered: