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

Fix black formatting failure on tokenize.pyi #513

Closed
wants to merge 1 commit into from

Conversation

stroxler
Copy link
Contributor

@stroxler stroxler commented Aug 17, 2021

Summary

I noticed this file was mal-formatted when using black . to
autoformat a different branch.

This was confusing to me, I rely on autoformatting after writing
messy code. The reason for it appears to be that black and isort
do not agree

To be specific:

  • even with profile = "black" set, it seems that isort will add only
    one blank line on .py files unless otherwise specified. This is in
    direct contradiction to what's stated in isort github issues and docs,
    which claim that profile = "black" should solve the problem on newer
    versions of isort (I tried against the latest and it makes no
    difference)
  • But if we use force blank_lines_after_imports = 2 as the
    pyproject.toml currently does (and is necessary to not mess up the
    majority of our code in *.py files, then isort and black will
    argue with one another about how many blank lines to use in .pyi files

My solution is to skip isort on *.pyi files, which I think is better
than having confusing behavior.

As an alternative, or in parallel, it might be worth me opening a PR
demonstrating that profile = "black" isn't working for us and
referencing it in an issue in isort.

Test Plan

python -m unittest
black .
isort .

I noticed this file was mal-formatted when using `black .` to
autoformat a different branch. Let's keep the autoformat clean.
@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Aug 17, 2021
@stroxler
Copy link
Contributor Author

Closing - this PR is correct in that black doesn't like our stubs file, but if I autoformat it then isort rejects the change.

Seems like a configuration issue, I can look into whether there's a way to make them agree

@stroxler stroxler closed this Aug 17, 2021
@stroxler
Copy link
Contributor Author

Reopening with a fix that disables isort on .pyi files

As discussed in the summary we could also try to raise this issue upstream - the isort team seems to think they closed this bug but their fix isn't working for me

@stroxler
Copy link
Contributor Author

On second thought, leaving closed, I think #515 is a better approach

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants