Question: feasibility of Black-formatting the code in this repo?
Opened this issue · 9 comments
Hi all,
this is "just" a request of your feelings in regards of running the Black formatter on the Python files in this repo?
We've been using it for some years now on our code and really like the benefits (particularly having very readable code and minimal diffs, independently of who's coding). Many recent development tools (VS Code, PyCharm, ...) are offering an integration for it, meaning they will also run the formatter whenever a file is being saved.
Having the scripts formatted by Black would IMHO facilitate interaction quite a bit (e.g. currently I would always have to explicitly ask VS Code to save the file without running the formatter in order to prevent it from producing a ton of changes). I'm even tempted to say this would make it more attractive for potential contributors, but that's obviously my very personal and strongly biased opinion 😁 😉
Now, obviously, I could just do this and file a PR - however, this would trick git and Github into believing that I am a major contributor to this repo as all the (black) changed lines would be attributed to my account. Therefore, even if you were positive about my suggestion / question, I would prefer someone else from the actual contributors to perform the change.
Happy to hear about your opinion. And just to be clear, a plain "no 🙅🏼♂️ we don't want this" is obviously fully acceptable!
Cheers,
Niko
Sounds good. 👍
I guess you want this checked in the github actions for open PRs, to make sure the formatting is maintained?
Before running black, any PR will need to be integrated. This will have a major impact on old code and will need to be discussed with active contributors like @Tom-TBT. This is why we do not run it on repository like omero-py.
Hello,
I am ok with running black too. I can help with reviewing the open PRs that are not mine.
In general, I am fairly supportive of any tools that improve code readability and quality and support the writing, review and inclusion of code contributions. I believe this was the spirit of introducing flake8 to the OMERO Python codebase many years ago.
On black, adding a few thoughts based on the experience of formatting the OMERO.web repository in ome/omero-web#218. @jburel has raised the operational impact on open contributions which typically need to be coordinated, put on hold and updated to fix conflicts. Post-formatting, it is also worth mentioning the impact on code maintenance. Running an existing repository through black can easily modify 1-10K lines of code depending on the size, also splitting lines to comply with the formatting rules. This introduces noise, clutters the version history and increases the complexity of any troubleshooting/maintenance task that requires to look at historical changes in order to track the introduction of a particular feature/bug.
Overall my reluctance has more to do with the scenario of formatting an existing codebase with significant history. For any new Python repository, introducing a formatting style does not carry the downsides mentioned above.
What @sbesson said.
As someone who is often trying to answer the question "why did we do this 10 years ago?" when pursuing low level bugfixes the history churn created by sweeping formatting changes makes that a lot more difficult. black is particularly bad for this because it is very opinionated on how it handles multi-line blocks and their relationship to the maximum line length.
Two quick points:
- .git-blame-ignore-revs and similar make the situation a bit better
ruffis the newblack;)
Thanks, @joshmoore. I need to update my git version apparently; you need at least 2.23.0 to use .git-blame-ignore-revs. Might give it a try on omero-web where there is a lot of historical reformatting mess in the history just to see how it behaves.
Very interesting indeed and it looks like the feature is already integrated in the GitHub UI - https://docs.github.com/en/repositories/working-with-files/using-files/viewing-a-file#ignore-commits-in-the-blame-view
An example from the zarr-python repo: zarr-developers/zarr-python#1459