PyCQA / isort

A Python utility / library to sort imports.
https://pycqa.github.io/isort/
MIT License
6.45k stars 574 forks source link

Delete unused imports #1105

Open astier opened 4 years ago

astier commented 4 years ago

Would it be possible to delete unused import?

anirudnits commented 4 years ago

Hi! if nobody is already working on this, I would like to give it a shot. I'm still in the process of going through the codebase and afterwards will be able, hopefully to come up with some way to get it done.

jugmac00 commented 4 years ago

Just as a note - flake8 already gives a warning about unused imports, so maybe it is not necessary to also include it in this package. In the spirit of do one thing and do it well.

anirudnits commented 4 years ago

@jugmac00 thanks. I think the issue is to remove the unused imports, which could be a nice little addition to isort especially in module(s) with a large number of imports.

jugmac00 commented 4 years ago

@anirudnits I just applied Flake8 (F401 Module imported but unused) to a large legacy code base - and yes, it was a bit of work to delete all unused imports by hand, but even Flake8 marked one import as unused although it was used.

I would not feel well with isort removing imports.

This is only my personal opinion. When unused imports get deleted automatically in future, I just can hope my test suite is good enough to find all false positives.

Update My test suite caught two more false positives from Flake8

anirudnits commented 4 years ago

@jugmac00 that's a valid concern and I do understand why this feature will negatively impact isort.

timothycrosley commented 4 years ago

I'm perfectly happy with isort doing this, as long as it requires a CLI flag, and can't be made the default via a config file. isort itself would break if its own "unused" imports were removed. But yeah, I can only see havoc happening if this could be put in a config, and happened automatically to everyone using precommit.

bersbersbers commented 3 years ago

Would it be possible to delete unused import?

What this would mean in any decent editor with automatic input sorting: you comment out the last line the requires an import, you save the file -> the import is gone. You uncomment the same line, you save the file -> the import does not come back, you are left with non-working code. That goes way beyond what I would expect i*sort* to do.

This definitely needs a default-off flag. Or maybe do this in a separate iclean binary?

timothycrosley commented 3 years ago

I really like the idea of a separate command to do it. Especially, since unlike isort itself, it would likely utilize flake8 or a different external library to identify the unused imports as others have already solved that problem well - isort's job would just be to make sure the format doesn't change during / after the removal.

lyz-code commented 3 years ago

Hi all, I've just created autoimport for this purpose. Issues and pull requests are welcome :)

ZeeD commented 3 years ago

While I agree that this could be a default-off option, I think that should be possible to set a "remove unused imports" flag in the config file. Keep in mind that the "action" of sorting import can broke code, so it's not really that different to "protect" the offending rows as the other false positive cases

Moreover at the moment I use isort in eclipse (through PyDev) with a specific keybinding (CTRL-SHIFT-o, or "Source"->"Organize Imports"). What it normally does is modify the imports - but it doesn't save, allowing the user to look at it and decide by himself. My worflow is something like add code, imports are imported automatically, do test, etc... delete unused code. save. "Organize imports" (look at the result) and eventually save again.

PeterJCLaw commented 3 years ago

You might be interested in a tool (fix8) I'm working on to automatically fix various flake8-highlighted errors; which includes support for removing unused imports. It's very rough around the edges at the moment, but having been using it myself for ~6 months I'm finding it really handy.

If there's interest in it, I've also got the beginnings of a VSCode plugin for it; not published yet though.

adzenith commented 3 years ago

autoflake will also remove unused imports using pyflakes (which is what flake8 uses to check for extra imports).

hadialqattan commented 3 years ago

In my opinion, there is no need to add this functionality while some advanced tools like pycln already exist to accomplish this task elegantly.

b-long commented 2 years ago

Replying to @hadialqattan 's comment (which I think is a valuable point).

Perhaps this feature could be enabled by an opt-in method (e.g. a command line flag), so that users that want the behavior have it and users who do not want the behavior aren't forced to use it.

micimize commented 2 years ago

IMO the main benefit for adding this feature to isort rather than leaving it to external libraries would be --diff support. For example, https://github.com/microsoft/vscode-python uses isort --diff for import organization.

Otherwise I think it's reasonable to just say "run autoflake; isort instead of isort; autoflake."

tommyjcarpenter commented 1 year ago

as a user of isort, black, flake8, and autoflake (and ale), I'd like to chime in and say I would prefer this be part of isort (optionally) since black has deprecated most of autoflake - the only reason we use autoflake is for this --remove-unused-imports. I understand if the default in isort is not to do this, but I don't see a great argument against it as opt-in. Especially since isort already respects pyproject.yaml, setting remove_unused seems like a no brainer (in terms of whether this would be a useful addition)

SXHRYU commented 1 year ago

I agree with people who say isort should have a default-off flag for deleting unused imports. This is my main tool for working with imports, while black handles the body of the file. From the top of my head I could conceive the solution is that first isort should sort the imports, while flagging unused ones. Then on the second loop delete flagged imports. Even if it costs, what, ~1 more second, I think people would tolerate this for having such a "supertool" exclusively for imports. (I'm not saying this solution is easy, just my 5 cents)

AvlWx2014 commented 1 year ago

I also think this is a useful feature for isort. At the moment my pre-commit workflow is something like:

As for "off-by-default" I'd like to advocate for this feature to be "on-by-default" instead. Since linters like flake8 report errors for unused imports by default I think it makes sense for removing them to be the default behavior for isort.

There are definitely instances where someone might want to leave unused imports around, but since imo that's the less-common case it shouldn't be the default behavior. Flake8 exhibits this same behavior where you have to explicitly disable the "unused imports" check if you know better than the tool that your import needs to stick around.

Glad to see others advocating for this feature as well!

sanmai-NL commented 1 year ago

Ruff can autofix this as well: https://beta.ruff.rs/docs/rules/unused-import/

jugmac00 commented 1 year ago

Ruff aims to be the all-in-one tool, so I think that is not a good comparison.

CodyCBakerPhD commented 6 months ago

Checking in for the year 2024 - any updates on this? Planned/not planned?

Would be great to have this feature in isort

8Observer8 commented 6 months ago

It is a very useful topic! Thanks, guys!

I just show to beginners how to use these tools that you mentioned above.

To sort packages in your project directory:

pip install isort
cd your_project_directory
isort .

To detect unused packages in your project directory:

pip install pyflakes
cd your_project_directory
pyflakes .