Skip to content
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

Style editions should be configurable #3539

Closed
Rapptz opened this issue Feb 1, 2023 · 10 comments
Closed

Style editions should be configurable #3539

Rapptz opened this issue Feb 1, 2023 · 10 comments
Labels
T: enhancement New feature or request T: style What do we want Blackened code to look like?

Comments

@Rapptz
Copy link

Rapptz commented Feb 1, 2023

Is your feature request related to a problem? Please describe.

Black recently released v23.1 which broke CI builds due to needing to reformat everything to the new "2023" style of code (see also: #3538)

Describe the solution you'd like

I would like to use v23.1 or higher and still maintain my old formatting to reduce churn and friction for new and old contributors to my project. This could be done using something like --style=2023 or --style=2022.

Describe alternatives you've considered

Pinning black in CI to a specific version. In my opinion this is unfeasible however, more on that below.

Additional context

With the formatting changes introduced, CI changes are bound to happen if you let yourself use an unbound (i.e. "latest") version of black. Some might say this is to be expected, because as per the stability policy you should instead pin the version to a "major" version instead to get the style.

This implementation of pinning to CI, however, is infeasible. The main friction comes from various contributors for different projects who now have to maintain different versions of black either in various virtual environments or know which version to install for which project. Development tools such as integrations with IDEs might also use a globally installed black executable. As more time goes on this friction of style changes will keep happening if we're unable to pick the style edition.

It is my understanding that the appeal of "black" was to not have to worry about formatting bikeshedding, but with the introduction of style editions this introduces another dimension of formatting bikeshedding.

Right now, these changes are backwards compatible so that code formatted with v23.1 when run with v22.6 will not incur changes to the output. I have only tested this with my project's configuration so I'm unsure if this holds true with the default set of features if no flags or configuration is given. However, from the stability policy there is no mention that these changes would end up being backwards compatible long term or something that is guaranteed.

@Rapptz Rapptz added the T: enhancement New feature or request label Feb 1, 2023
@JelleZijlstra
Copy link
Collaborator

If you don't want formatting to change, you should pin to a specific year's versions, like black ~= 22.0. You can set required_version = "22" in your configuration to ensure that all contributors use this version.

It is not feasible for us to support multiple style options long term. We are a small team of maintainers and supporting multiple styles would add significant complexity to the code we have to maintain.

@JelleZijlstra JelleZijlstra closed this as not planned Won't fix, can't repro, duplicate, stale Feb 1, 2023
@Rapptz
Copy link
Author

Rapptz commented Feb 1, 2023

I already mentioned why pinning a version to CI doesn't scale very well. Mainly because we're now expected to continuously update our style every year for these minor churn commits that are at the whim of the black maintainers. Personally, this wouldn't be a problem for me if the style itself was more configurable but since black advertises itself as a no-configuration opinionated formatter then having this opinion change yearly is too much friction.

With all due respect, I can sympathise with having a small team of contributors to work on this but similar sized teams have managed to make other formatters as well. autopep8 is maintained by far fewer people with fewer contributors, and the same can be said about YAPF. The difference is that these smaller projects also have a lot more configuration than black.

@mikeshardmind
Copy link

With all due respect, I don't see a format that is opinionated and changes that opinion every year as useful, nor does that yearly change adhere to one of the reasons I adopted black in some of my projects early on. One of the biggest selling points of black (when I adopted it for my own use) was that it minimized code churn with the style it enforced.

It hasn't fully lived up to this, even prior to now, but seeing this issue prompted actually expressing it.

Yearly churn commits which reformat a project create noise in other useful tools like git blame, and lack of updating runs into all the issues already raised above.

@ichard26 ichard26 added the T: style What do we want Blackened code to look like? label Feb 1, 2023
@hauntsaninja
Copy link
Collaborator

hauntsaninja commented Feb 2, 2023

I already mentioned why pinning a version to CI doesn't scale very well

It was easy to skip over, but just noting that Jelle proposed setting required_version in your pyproject.toml, which is very useful and is the easy half of what you want. This serves as a nice guardrail if contributors mess up their versions. (And of course, there are plenty of solutions to help contributors get the right versions of packages; this is a very general problem that every software project faces)

I'll also mention that forking to make zero changes ever is about as maintainable as a hard fork can get. Maybe someone here would like to make a black22 project and keep it working for forever?

@cooperlees
Copy link
Collaborator

I love the suggestion by @hauntsaninja. black is available for you all to fork and make it work how you want! I also want to say I second everything @JelleZijlstra shares. We are an evolving opinionated formatter. As Python's syntax and features evolve, we will too.

@Rapptz
Copy link
Author

Rapptz commented Feb 3, 2023

Fundamentally, these suggestions are missing the point that I'm making. I want there to be less friction for my contributors when contributing to my project. Requiring a specific version of black or even a fork of black doesn't play well with contributors when they just have either an IDE set-up black or a global black install. We can make the argument that they should be using a venv for all their devtooling anyway but that's an ideal world that I don't live in. My goal is to reduce friction for new contributors and old contributors, and these suggestions don't help with that.

@JelleZijlstra
Copy link
Collaborator

I'm not missing the point (and I commend you for trying to make your contributors' lives easier), I'm just disagreeing that this is a reasonable expectation. Black's 2022 stable style existed only for one year. In the first four years of Black's existence (2018–2021), we could and did change the formatting style in every release. When we switched to non-beta releases in early 2022, we communicated clearly that the stable style would remain stable for one year only.

Black is no different here from any other development tool: if your contributors run a different version of flake8, or mypy, or prettier than you do, they are similarly going to have different output. A workaround can be to use pre-commit.ci to automatically push Black re-formatting commits to PR branches, so that contributors don't need to worry about having Black set up locally. We have good experience with this setup at typeshed.

@ichard26
Copy link
Collaborator

ichard26 commented Feb 3, 2023

FTR, while I too would like Black's style to be stable, it is simply unpractical and unreasonable to never make changes to it. Bugfixes aside, there are some improvements to the style we would like to eventually implement.

I consider the stability policy to be an improvement from the old status quo. As Jelle mentioned, before 22.0.0, there were zero formal guarantees about style stability.

[...] maintain different versions of black either in various virtual environments or know which version to install for which project.

Isn't this what developer documentation, requirement files (or equivalent) or environment provisioning tools like tox, nox, hatch, or poetry (and pre-commit in a way) to suppose to deal with? Yes, it's friction, but Black isn't special here. Other development packages can break or introduce/remove a feature that makes certain versions unusable.

However, I do see how global (editor) installs can be a nightmare and yearly style updates can be surprising and frustrating. So... I have a proposal:


The pitch

We introduce a --style-edition option (exact name TBD) that configures which year's stable style is used as suggested in the OP.

If the configured edition is unavailable, Black will error out.

Implementation

The black package code from the (latest version1) of an older style edition will be vendored wholesale under a __black$year package. This would be done for all of the included older editions for a release, along side the normal black package and its (default) edition. At runtime, using either explicit imports or some custom importlib machinary, the correct format function (probably black.format_str()) is used to format using the configured edition.

This is complicated by the fact that Black's safety checks are somewhat dependant on the specifics of the formatting, so the safety check functions would also probably have to be chosen at runtime.

There are questions about how black.Mode() / style configuration forward and backward compatibility would be dealt with. For now I'm not considering this as I don't expect this proposal to be accepted anyway.

To keep the maintenance burden low, this would be at least partially automated. The vendoring (including rewriting the imports of the vendored code), testing, and removal of the older editions would ideally be all automated. At the bare minimum, helper scripts that do the bulk of the work for this must exist. One sticking point is that Black would need to know the default edition and the extra editions included. For the default edition, checking the version at runtime will be too unreliable so it would have to be hardcoded. For the extra editions, hardcoding is the only option.

Limitations

  • Only the current stable style will be actively maintained and supported. No bugfixes, performance improvements, or other changes will be made to previous editions (barring bugfixes to keep the code not-crashing I guess). They will remain "frozen in time" for upmost stability (until they're removed; see next point) and our (us, the maintainers') sanity.

  • After a style edition is no longer the latest, it will remain accessible for more 2-5 years (exact duration TBD) and then it will be dropped from the next major release.2 So for example, --style-edition=2023 would be supported until Black 25.1.0 or 28.1.0. If you'd like to use that version or newer, you must switch to a more modern style.

  • If --style-edition is not set to the latest edition (that ships with the release), the --preview flag cannot be used. While I can't think of any immediate technical issues3, keeping the configurability as low as needed is preferable. If you're using an older style, you probably don't want the potentially "rough around the edges" preview style of that year anyway.

  • The code for the older editions will not be compiled by mypyc. If you want the best performance, you must use a release where the wanted edition is the default. I don't want Black's wheels to grow 3-6x in size just so every included edition is fast (and neither do I want to make sure the older code still compiles fine, fixing any new type errors that crop up).

My opinion on this idea

Even though it's mine, I don't like it much as it's introducing more configurability in a way4, but if there's significant user feedback suggesting this is needed, it's probably the least bad option and something I could consider. As of right now, I do not consider 13 👍s to be enough.

Also, this is just my personal suggestion. This would need approval from other maintainers before being accepted.

Footnotes

  1. Sometimes we make bugfixes to the stable style which end up changing it a tiny bit. We don't do this often and we only do this if there's severe bug (like Black straight up deleting comments) but it does happen.

  2. While I guess we could just never remove old editions, that would eventually lead to breakage that we would have to deal with. Future Python versions can always break things (even if Black's core formatting logic is rather version independent). Also less choice is preferable in general.

  3. ... although I'd probably think of some if I thought more about this. Many ideas that seem simple and good on first thought will turn out be quite complex once you start implementing them IME.

  4. Yes, you can argue that this kind of configurability already exists by simply pinning Black to an older release. However, it is way easier when you don't even need to install a specific version of Black and can simply set an option. And the easier it is change formatting, the more likely bikeshedding is going to occur. (e.g. "What edition should we use?")

@Rapptz
Copy link
Author

Rapptz commented Feb 3, 2023

Thanks for the response, I appreciate it.

FTR, while I too would like Black's style to be stable, it is simply unpractical and unreasonable to never make changes to it. Bugfixes aside, there are some improvements to the style we would like to eventually implement.

I'm okay with styling changes that are necessary due to bug related reasons, those are more than fine! I'm not really expecting things to be static until the end of time and I'm well aware that bugs can happen. I considered these editions to be different because they're more so a larger category of changes that are applied all-at-once when a new yearly release comes out irrespective of bugfixes.

I consider the stability policy to be an improvement from the old status quo. As Jelle mentioned, before 22.0.0, there were zero formal guarantees about style stability.

I'm appreciative of stability policies, however I have one minor nit with it that I mentioned in the OP about these not being guaranteed to be forward/backward compatible. Or at least an attempt at it. For example right now if I format with 23.1.0 and then someone else reformats using 22.6 then it works out okay with the latter presenting no changes but this isn't actually guaranteed (or even mentioned that there's a best-effort to do this).

However, I didn't adopt black onto my projects until early 2022 due to mostly pressure from contributors who had configured "format on save" on their editor causing excessive formatting changes.

Yes, it's friction, but Black isn't special here. Other development packages can break or introduce/remove a feature that makes certain versions unusable.

These tools aren't formatting tools which directly impact the code's presentation (causing git merge conflicts or excessive changes) so I consider them different.

As for the pitch itself, I'm supportive of it. I'm not particularly expecting maintenance of old style editions forever so having it no longer work within a few years is more than fine for me.

@felix-hilden
Copy link
Collaborator

felix-hilden commented Feb 3, 2023

Also leaning on the side of not doing anything here. Requirements pinning, required_version and CI make it very reasonable to enforce and check a specific style. And contributing experience can be made better with automation to a high degree. As Jelle said, the stability policy was a step towards a more predictable pace with formatting changes already. We have over a hundred open design issues, so Black is far from complete even if nothing was added to Python ever again.

I'm willing to entertain Richard's suggestion, but it doesn't seem that simple to me either, especially compared to using CI. It is readily available, and probably already in place if projects really want to enforce a style rather than accepting whatever the user happened to run on the code locally.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T: enhancement New feature or request T: style What do we want Blackened code to look like?
Projects
None yet
Development

No branches or pull requests

7 participants