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

Import sorting deletes comments #675

Closed
andersk opened this issue Nov 11, 2022 · 5 comments · Fixed by #749
Closed

Import sorting deletes comments #675

andersk opened this issue Nov 11, 2022 · 5 comments · Fixed by #749
Assignees
Labels
isort Related to import sorting

Comments

@andersk
Copy link
Contributor

andersk commented Nov 11, 2022

(This is a known limitation mentioned in #465 (comment), but might as well be tracked as an issue.)

ruff --select=I --fix incorrectly transforms

# preserved
import os  # deleted

# deleted
import third_party  # deleted

into

# preserved
import os

import third_party
@charliermarsh charliermarsh added the isort Related to import sorting label Nov 11, 2022
@charliermarsh charliermarsh self-assigned this Nov 12, 2022
@charliermarsh
Copy link
Member

I don't understand isort's comment behavior yet, trying to make sense of it.

Before:

# Comment 1
# Comment 2
import D

# Comment 3a
import C
# Comment 3b
import C

import B  # Comment 4

# Comment 5

# Comment 6
from A import (
	a,  # Comment 7
	b,
	c,  # Comment 8
)
from A import (
	a,  # Comment 9
	b,  # Comment 10
	c,  # Comment 11
)

After:

# Comment 1
# Comment 2
import B  # Comment 4

# Comment 3b
# Comment 3a
import C
import D

# Comment 6
from A import a  # Comment 9
from A import b  # Comment 10
from A import c  # Comment 11

# Comment 5

@charliermarsh
Copy link
Member

I'll just try to come up with something reasonable that does the right thing in the common cases (comment at end-of-line, etc.).

@charliermarsh
Copy link
Member

Gonna try to fix this today. I've added comment extraction, now I just need to figure out the preservation strategy.

@tiangolo
Copy link

Amazing! This is one of the two things not allowing me to switch to Ruff's sorting, I need to preserve some comments for some # type: ignore and things like that.

@charliermarsh
Copy link
Member

This took longer than expected. I'm not super happy with the amount of code required to handle comments, but the output is looking pretty good.

Here's the diff on Zulip:

Screen Shot 2022-11-15 at 9 27 05 PM

Here's the diff on FastAPI:

Screen Shot 2022-11-15 at 9 27 48 PM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
isort Related to import sorting
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants