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

isort removes newlines before standalone comments if the following line is an import of the same group #251

Closed
gaborbernat opened this issue May 23, 2018 · 13 comments
Labels
T: enhancement New feature or request

Comments

@gaborbernat
Copy link
Contributor

gaborbernat commented May 23, 2018

Operating system: Arch Linux
Python version: Python 3.6.6
Black version: 18.5b0
Does also happen on master: yes

Given:

import a
# noinspection PyProtectedMember
import _a

Black formats this to (note the introduced empty line):

import a
# noinspection PyProtectedMember

import _a

However isort correctly (in the recommended mode 3) formats it to:

import a
# noinspection PyProtectedMember
import _a
@ambv
Copy link
Collaborator

ambv commented May 24, 2018

Black puts an empty line after every group of imports. A standalone comment signifies an end of a group of imports before it.

So, your example is wrong, Black will do this instead with the original file:

--- /tmp/asd.py  (original)
+++ /tmp/asd.py  (formatted)
@@ -1,4 +1,5 @@
 import a
+
 # noinspection PyProtectedMember
 import _a

The behavior between isort and Black is indeed different here. I haven't decided yet how to resolve this.

@gaborbernat
Copy link
Contributor Author

Thanks for the explanation, would be nice to somehow support this use case though.

@ambv ambv changed the title comments in imports incorrectly generate empty line isort removes newlines before standalone comments if the following line is an import of the same group May 29, 2018
@ambv ambv added the T: enhancement New feature or request label May 29, 2018
@skorokithakis
Copy link
Contributor

I am affected by this as well :/ Just to clarify, what is the problem here? Is Black wrong, or is isort? Is the newline supposed to be there?

@gaborbernat
Copy link
Contributor Author

gaborbernat commented Jun 26, 2018

@ambv any plans to address this? This makes the line of This also makes Black compatible with isort with the following configuration. from the readme not entirely true. And the only solution in the meantime is to remove such comments, which is not good generally. I get that black does not want to venture into the business of import ordering; but in that case it should play nice with tools out there that do this task, e.g. isort. I personally would place this as a pretty critical bug; not enhancement per se. If there's a decision on this maybe we (the community) can try to create a PR, but for that, we would need to discuss what a solution would be.

@tibz7-zz
Copy link

I'm also affected. what is the current status about this bug?

@thisfred
Copy link

thisfred commented Dec 31, 2018

I think black is being too prescriptive here: yes comments between imports can be used as headers, but they can also be things like:

import a
# XXX: unused import, that however shouldn't be removed 
# because our framework depends on it being present
import b

Pierre-Sassoulas added a commit to Pierre-Sassoulas/pylint that referenced this issue Mar 9, 2019
Apparently black and isort do not behave the same.
See : psf/black#251
Pierre-Sassoulas added a commit to Pierre-Sassoulas/pylint that referenced this issue Mar 19, 2019
Apparently black and isort do not behave the same.
See : psf/black#251
PCManticore pushed a commit to pylint-dev/pylint that referenced this issue Mar 20, 2019
Apparently black and isort do not behave the same.
See : psf/black#251
@Sharadh
Copy link

Sharadh commented Jun 3, 2019

Hello @ambv ! Do you have an update on this?

The behavior between isort and Black is indeed different here. I haven't decided yet how to resolve this.

Perhaps you can talk through your thoughts so we understand if a fix is likely?

Also, re:

Black puts an empty line after every group of imports. A standalone comment signifies an end of a group of imports before it.

Is there some documentation that describes this behavior? I couldn't find it in the code style - I ask because without knowing the reasoning behind this behavior, I find it hard to suggest a way forward. I do find the rest of the decisions well documented, and do a great job of answering my questions and feature requests before I make them ;)

EgorChernik added a commit to EgorChernik/black that referenced this issue Jun 25, 2019
…if the following line is an import of the same group
@Sharadh
Copy link

Sharadh commented Jun 26, 2019

Does anyone know what the best way to get a contributor's attention here is? I'm happy to submit a proposal + fix, it's just that the radio silence makes me nervous about investing time :)

@roushan05
Copy link

roushan05 commented Aug 12, 2019

Any update on this ? What is the current status of this issue ? Not sure which way to proceed here .... Black or isort ?

FuegoFro added a commit to discord/isort that referenced this issue Aug 18, 2019
This adds a `ensure_newline_before_comments` configuration that matches the format that `black` imposes (for better or worse) and allows `isort` to be used with `black` without any thrashing/conflicts with regards to blank lines before comments.

Should help address psf/black#251
@hugovk
Copy link
Contributor

hugovk commented Sep 25, 2019

This is now fixed in isort by adding a new ensure_newline_before_comments option, but it's not yet released:

If you use pre-commit, you can pin to the latest isort commit, like this:

cjp256 pushed a commit to cjp256/snapcraft that referenced this issue Nov 22, 2019
Nearly perfect match config to black formatting with one exception:
psf/black#251

Signed-off-by: Chris Patterson <chris.patterson@canonical.com>
cjp256 pushed a commit to cjp256/snapcraft that referenced this issue Nov 22, 2019
Nearly perfect match config to black formatting with one exception:
psf/black#251

That gap should be filled with the 'ensure_newline_before_comments=True'
that won't be honored until the next version of isort.

Signed-off-by: Chris Patterson <chris.patterson@canonical.com>
yelizariev pushed a commit to yelizariev/itpp-runbot that referenced this issue Feb 11, 2020
srittau added a commit to srittau/typeshed that referenced this issue Jun 11, 2020
A few comments between imports were removed or moved to the top of the
import block, due to behavioral differences between black and isort. See
psf/black#251 for details.

In two instances @Overloads at the top of the file needed to be moved
due to psf/black#1490.
srittau added a commit to srittau/typeshed that referenced this issue Jun 14, 2020
A few comments between imports were removed or moved to the top of the
import block, due to behavioral differences between black and isort. See
psf/black#251 for details.

In two instances @Overloads at the top of the file needed to be moved
due to psf/black#1490.
JelleZijlstra pushed a commit to python/typeshed that referenced this issue Jun 14, 2020
A few comments between imports were removed or moved to the top of the
import block, due to behavioral differences between black and isort. See
psf/black#251 for details.

In two instances @Overloads at the top of the file needed to be moved
due to psf/black#1490.
benjaminhwilliams added a commit to xia2/screen19 that referenced this issue Jun 23, 2020
Comments in import blocks cause disagreement between Black and isort.
This will soon be fixed by specifying
'ensure_newline_before_comments=true' in the isort config, though this
won't be honoured until the next isort release.

See psf/black#251
and PyCQA/isort#1000
benjaminhwilliams added a commit to xia2/xia2 that referenced this issue Jun 23, 2020
Comments in import blocks cause disagreement between Black and isort.
This will soon be fixed by specifying
'ensure_newline_before_comments=true' in the isort config, though this
won't be honoured until the next isort release.

See psf/black#251
and PyCQA/isort#1000
benjaminhwilliams added a commit to dials/dials_scratch that referenced this issue Jun 23, 2020
Comments in import blocks cause disagreement between Black and isort.
This will soon be fixed by specifying
'ensure_newline_before_comments=true' in the isort config, though this
won't be honoured until the next isort release.

See psf/black#251
and PyCQA/isort#1000
benjaminhwilliams added a commit to dials/dials that referenced this issue Jun 23, 2020
Comments in import blocks cause disagreement between Black and isort.
This will soon be fixed by specifying
'ensure_newline_before_comments=true' in the isort config, though this
won't be honoured until the next isort release.

See psf/black#251
and PyCQA/isort#1000
benjaminhwilliams added a commit to cctbx/dxtbx that referenced this issue Jun 23, 2020
Comments in import blocks cause disagreement between Black and isort.
This will soon be fixed by specifying
'ensure_newline_before_comments=true' in the isort config, though this
won't be honoured until the next isort release.

See psf/black#251
and PyCQA/isort#1000
vishalkuo pushed a commit to vishalkuo/typeshed that referenced this issue Jun 26, 2020
A few comments between imports were removed or moved to the top of the
import block, due to behavioral differences between black and isort. See
psf/black#251 for details.

In two instances @Overloads at the top of the file needed to be moved
due to psf/black#1490.
@hugovk
Copy link
Contributor

hugovk commented Jul 3, 2020

This is now fixed in isort by adding a new ensure_newline_before_comments option, but it's not yet released:

Looks like isort 5.0.0 is scheduled for tomorrow!

@skorokithakis
Copy link
Contributor

By the way, I have switched to https://github.com/asottile/reorder_python_imports, which doesn't have this problem and runs statically without isort's troubles when the sorted packages aren't installed.

@ichard26
Copy link
Collaborator

Closing as isort 5 has been released.

Either use isort 5's profiles of which one is built for Black, or manually configure ensure_newline_before_comments to true, although it only works with isort 5 and higher. For more information please read the documentation for isort + Black compatiblity.

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
Projects
None yet
Development

No branches or pull requests

9 participants