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

Use Black profile #17

Merged
merged 1 commit into from
Oct 5, 2020
Merged

Conversation

timothycrosley
Copy link
Contributor

@timothycrosley timothycrosley commented Oct 2, 2020

As a possible alternative to: #16. Updates project to use the black profile for isort. Additionally, removes some unnecessary skips that were present.

Independent of the approach you decide, one benefit of placing the configuration outside of pre-commit is that if you or a contributor uses a text editor that formats on save, there wont be unnecessary churn when a commit is made.

@jugmac00
Copy link
Owner

jugmac00 commented Oct 2, 2020

@timothycrosley Thanks for this pr!

I guess at this point both tools are an equally good fit, which means at some time I have to decide which one to use :-)

P.S.: Thanks for the hint to move the black config into the pyproject.toml. I did not think of the possibility that others could use e.g. black via their IDE. Good catch! But on the other hand - maybe they use another version of black, and then the output does not match, again.

While you can pin versions also via requirements.txt pip.file(?), poetry lock files, buildout.cfg and what not, I really like the approach via pre-commit, e.g pin the version and specify how to tool is run.

P.P.S Thanks for your pr and for all the work you put in isort. I am a member of both Zope and Plone foundation, and we use isort literally in hundreds of repositories!

@timothycrosley
Copy link
Contributor Author

I guess at this point both tools are an equally good fit

One thing I will note, is that isort is doing a lot more sorting for you, that you won't notice in this kind of comparison because isort already sorted the imports. However, overtime if isort is no longer used they will likely drift. reorder-python-imports stops sorting at the first line of non-import code (such as a try: or an assignment) so it misses a lot of imports. https://github.com/jugmac00/Products.ZopeTree/pull/16/files#diff-2b1e7ab368801d3f6df6cb0b8c4ec91fR17 is a good example of this. That import is against the reorder-python-imports style which enforces one import per a line, but is not updated because there was an assignment above it. This file is a particularly good example, as it shows a case where isort will keep a total of 4 imports sorted and styled for you, while reorder-python-imports will only "reorder" the one import.

P.S.: Thanks for the hint to move the black config into the pyproject.toml. I did not think of the possibility that others could use e.g. black via their IDE. Good catch! But on the other hand - maybe they use another version of black, and then the output does not match, again.

It's true that right now black is too inconsistent between releases at the moment for it to be of much value. Though I hope and feel confident this is a temporary situation that will change once it is stable. isort has a formatting guarantee https://pycqa.github.io/isort/docs/major_releases/release_policy/#formatting-guarantees that provides some confidence that if different versions are used, they should produce the same output with the same settings (minus bug fixes, or if you want to use a bleeding edge feature - such as isort's ability to sort literals and assignments). For projects that use isort, as much as possible we've been adding them to an integration test suite to make sure this is the case and no regressions are introduced. Black does the same, however, for now it mostly ignores formatting changes that occur between versions as they want the freedom to make those changes before becoming stable.

P.P.S Thanks for your pr and for all the work you put in isort. I am a member of both Zope and Plone foundation, and we use isort literally in hundreds of repositories!

Thanks, this means a lot to me! I'm glad you you've had success with it!

@jugmac00 jugmac00 merged commit 3542e2f into jugmac00:master Oct 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants