Pyomo / pyomo

An object-oriented algebraic modeling language in Python for structured optimization problems.
https://www.pyomo.org
Other
1.97k stars 507 forks source link

Make the entire code base PEP8 compliant. #329

Closed carldlaird closed 1 year ago

mrmundt commented 1 year ago

IDAES recently implemented the usage of black to enforce PEP8 compliance; in interest of compatibility, I would like to implement the same. I intend to run black on the entire codebase after the following are merged:

jsiirola commented 1 year ago

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 .git-blame-ignore-revs file (see here)

mrmundt commented 1 year ago

I intend to do the black application in chunks so as to not make it a reviewing nightmare.

Also, for posterity, I intend to run this command: black -S <path> <- The -S skips string normalization.

blnicho commented 1 year ago

Double check these files to make sure the line numbers are correct after applying black

The problematic line was replaced in #2754

bernalde commented 1 year ago

Will black also affect contrib directories?

blnicho commented 1 year ago

@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.

bernalde commented 1 year ago

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

blnicho commented 1 year ago

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.

mrmundt commented 1 year ago

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.

mrmundt commented 1 year ago

Adding another comment for posterity:

mrmundt commented 1 year ago

For closure: