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

Update isort config to use_parentheses instead of combine_as_imports #547

Merged
merged 1 commit into from
Nov 8, 2018

Conversation

cas--
Copy link
Contributor

@cas-- cas-- commented Oct 2, 2018

The combine_as_imports=True modifies isort style as a side-effect and was not the intended purpose of the suggested change in #250. The problem was that isort was actually replacing the parens with backslash and using combine_as_imports=True happened to also produce the same result.

The actual setting should be use_parentheses as this tells isort to use parenthesis for line continuation instead of \ for lines over the allotted line length limit and matches precisely what black is outputting.

The `combine_as_imports=True` modifies isort style as a side-effect and was not the intended purpose of the suggested change in psf#250. The problem was that isort was actually replacing the parens with backslash and using `combine_as_imports=True` happened to also produce the same result.

The actual setting should be `use_parentheses` as this tells isort to use parenthesis for line continuation instead of \ for lines over the allotted line length limit and matches precisely what black is outputting.
@coveralls
Copy link

coveralls commented Oct 2, 2018

Pull Request Test Coverage Report for Build 773

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 31 unchanged lines in 3 files lost coverage.
  • Overall coverage increased (+0.2%) to 96.168%

Files with Coverage Reduction New Missed Lines %
blackd.py 2 97.22%
tests/test_black.py 4 97.83%
black.py 25 95.19%
Totals Coverage Status
Change from base Build 772: 0.2%
Covered Lines: 2911
Relevant Lines: 3027

💛 - Coveralls

@cas--
Copy link
Contributor Author

cas-- commented Oct 22, 2018

Can this be reviewed for merging?

@cas--
Copy link
Contributor Author

cas-- commented Nov 1, 2018

@ambv Please can you review this PR

@cas--
Copy link
Contributor Author

cas-- commented Nov 8, 2018

Anyone?

@ambv ambv merged commit 158f796 into psf:master Nov 8, 2018
@ambv
Copy link
Collaborator

ambv commented Nov 8, 2018

Thanks! ✨ 🍰 ✨

jleclanche pushed a commit to jleclanche/tan that referenced this pull request Nov 14, 2018
…sf#547)

The `combine_as_imports=True` modifies isort style as a side-effect and was not the intended purpose of the suggested change in psf#250. The problem was that isort was actually replacing the parens with backslash and using `combine_as_imports=True` happened to also produce the same result.

The actual setting should be `use_parentheses` as this tells isort to use parenthesis for line continuation instead of \ for lines over the allotted line length limit and matches precisely what black is outputting.

(cherry picked from commit 158f796)
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.

3 participants