[Feature Request] Optimize imports

[devxpy]

Can we please have a feature, similar to PyCharm's "optimize imports" feature, which basically sorts and removes un-used import statements?

P.S. Great work!, Black works like a charm for me.

[ghost]

Remove duplicate import also requested.

[hugovk]

For the time being, you can use isort to sort imports and autoflake to remove unused imports.

isort -rc .
autoflake -r --in-place --remove-unused-variables .

[jensens]

IMO import sorting should be handled separate outside of Black. With ISort (and others) we already have great tools.

[asottile]

https://github.com/ambv/black/issues/363#issuecomment-398261516 also serves as a decent indicator that this won't / shouldn't happen in black.

from @ambv's comment:

Black is not a linter, it's a formatter. It starts and ends with the same AST

adding / removing / sorting imports would change the ast

[tweakimp]

I wish there was something like black but for linting and code review. Maybe name it white? 😋

[denizdogan]

Obviously we can use isort and autoflake (or some other tools) to achieve the same thing, but a big benefit of Black is that it's opinionated and covers everything in one tool and one command and one optional configuration.

[ambv]

I'm warming up to the idea that Black should cover this use case too.

[tweakimp]

Which one? Sorting? Removing unused? Removing doubles?

[jaroel]

@ambv I understand why you'd want to, but pretty please stick to formatting only. Not only should one tool do one thing (tm), we'd also be wasting the years of effort and tuning that went into tools like isort, pyflakes, pylint, et all - and at the integrations with IDEs - their options, switches and nuances.

Linting is a different kind of beast from formatting. I'm really comfortable with letting black format my code, as it doesn't change the working of it. But you have no idea why my code was written as it is. For example: I need to have setup some paths before importing code - that's a given:

from configtype.jsonconfig import setup_search_paths
setup_search_paths(".")
from ocyan.main.settings import *  # isort:skip

I would expect that black-the-linter would always put all the imports together, without an cop-out:

from configtype.jsonconfig import setup_search_paths
from ocyan.main.settings import *
setup_search_paths(".")

I love that black takes out all discussion on formatting, but please please do not make black do linting (for lack of a better word).

Thank you for reading :)

[JelleZijlstra]

I would like to see Black auto-sort imports, actually. For my team the value of Black is in taking care of boring, unimportant code-related decisions in a consistent way, so that developers don't have to worry about these things. That value gets even bigger if we can have a single tool taking care of all of these decisions, instead of having one for formatting, one for sorting imports, etc. Otherwise, developers have to remember to run every one of those tools on their code, and it gets harder to get people to install editor plugins to run every tool automatically.

It's true that import sorting is a bit trickier than what Black currently does, because it will change the AST. To @jaroel's concern, we should limit import sorting to contiguous blocks of imports: if there is any non-import code, we should not move imports around it. Similarly, we should be careful not to reorder imports that define the same name, including import *s. We should also study how import sorting is done by existing tools like isort and learn from them.

[asottile]

Otherwise, developers have to remember to run every one of those tools on their code

fwiw, pre-commit goes a long way towards attempting to solve this problem. It makes it easy to set up as many linters as you want (in a variety of languages) and runs them automatically (commit / push time and/or through CI).

I myself am leaning towards @jaroel's sentiment. black should aim to do formatting well and leave import sorting to the battle tested import sorting tools. Reinventing isort doesn't seem productive

[jaroel]

@JelleZijlstra It looks to me that you want automation, not import-sorting-by-black per se :) The usual approach would be to wrap tools in a utility script, or use a Makefile to call everything in the correct order - or using a pre-commit hook. ie, make lint calls isort and then black. This makes it even easier for your developer, as they won't even need to know about black anymore ;)

In other words: I wouldn't want my car mechanic to clean my clothes, even if it would be handy to have that if I'm already waiting at the shop ;)

[zsol]

One practical problem we're running into for large codebases is that isort and black are slightly incompatible with each other, so running them after each other changes each file in the repository twice. Ideally, isort would have a black-compatible mode (or vice versa, but we have some reservations about isort's choice of style), but it looks like the maintainers of isort were not very responsive to https://github.com/timothycrosley/isort/issues/694

[ambv]

@jaroel, you raise some valid points. The very reason I'm considering sorting imports in Black is that isort does it wrong:

• In general, its implementation is problematic. Everything interesting happens in an __init__() method; it's based on string matching and not a syntax tree, and so on.
• Its configuration based on magic discovery of first-party/third-party code only works on activated virtualenvs, making it brittle for CI.
• It moves comments around in a way that is incompatible with Black.
• However, the killer thing for me is the example you give around imports placed after function calls being moved above the calls. Black would never do that because there are simply too many cases where this breaks the file. This makes codemodding any larger codebase unsafe.

Even if isort's author agreed with some of the above, backwards compatibility concerns pretty much force him to keep it as is.

If I followed some of the advice here around "reinventing" things, Black wouldn't see the light of day at all. We've had autopep8 and YAPF the entire time, didn't we? Just as I didn't want to write an auto-formatter, I don't want to creep more scope into it. Sadly, sorting imports looks increasingly inevitable.

Don't worry though. This feature requires careful planning so it won't appear out of the blue any time soon.

[asottile]

If I followed some of the advice here around "reinventing" things, Black wouldn't see the light of day at all. We've had autopep8 and YAPF the entire time, didn't we?

yapf/autopep8 have much more timid goals than black, not sure it's fair to compare them :) (whereas an import sorter would hopefully have identical goals).

That said, I also reinvented the isort wheel for exactly the reasons listed above (1 2 4) 🙃 (whoops I'm a hypocrite)

I definitely think (1) is fixable upstream. I've proposed fixes to (2) but this seems to be a sticking point (worked around instead), and (4) I just don't have any good excuses for isort -- this behaviour is unacceptable.

[digitalresistor]

@ambv is there anything we can do to help move this part of black along? Having fallen into various pitfalls of isort due to an issue of it's configuration (it merges .isort.cfg with ~/.isort.cfg for example, meaning a projects .isort.cfg has to define ALL keys or a users ~/.isort.cfg may override it), I'd love to see this implemented in black.

[JelleZijlstra]

@bertjwregeer I think the main limiting factor is that somebody has to actually do the work. Here's a list of some things that have to be taken care of:

• Generally review what other current tools (isort, and I'm sure there are others) do. What kind of things to they change? What style do they choose? How to they decide what is safe to change?
• Decide what the style is going to be that Black will enforce. Some questions to answer: Will we separate imports into different "sections", like internal vs. stdlib vs. third-party? How will we decide which imports go into which section? (I wouldn't want Black to go spelunking around the file system to figure out what is a third-party package, like isort apparently does.) Will we turn from x import y; from x import z into from x import y, z? (I would be OK with that but maybe it's a bridge too far.)
• Figure out what the safety features are that we need. @ambv and I mentioned a few above, like not moving imports around comments or function bodies, and not moving things around import *, but do we need more?
• We need to actually implement this in Black itself. We'll have to add a flag to skip import normalization. Import sorting will break Black's current safety check (which checks that the AST matches before and after), so we'll have to decide what to do about it (disable it or make it smarter).

[jaroel]

I've got a Zope/Grok project where we import the whole module and refer to that. ie import z3c.form.widgets + z3c.form.widgets.select instead of from z3c.form.widgets import select. It would be nice if that could be supported in some way?

[jensens]

In the @plone project we decided to sort imports alphabetical. Major reason is, if there are many imports it is easier to read and to find an import, without guessing what it is semantically. An we often have many imports. Also, the internal vs. third-party is difficult to define in a project with many packages. Is the application server code third party or not? Developing an Addon for Plone, is Plone then thirdparty? If it is taken into Plone-Core, do we then need to resort imports? There is no semantic true way to make sections nor is there a good way to automate that.

Alphabetical imports are simple, easy to read and also do not need code inspection for sorting.

It still breaks the AST diff. I usually do an isort and the run black.

The Black compatible (and currently still on a line length of 79) Isort configuration.

[hugovk]

@asottile's https://github.com/asottile/reorder_python_imports "has a single aim: reduce merge conflicts" and favours:

from typing import Dict
from typing import List

over:

from typing import Dict, List

[digitalresistor]

I much prefer grouping things together than a single import per line, I find the explosion of import lines to be distracting... but that all comes down to preference.

[tweakimp]

I think you can call it more than preference because if there is no other difference, less lines is better. in hugovks example the second version is shorter, easier to read and should be preferred in my opinion.

[asottile]

I have no stake in black's implementation, that said it's not just a stylistic preference. -- much like the rest of black there's a reason for its opinionated decisions. Many of them are similar to the rationale for the tool I've written: avoid merge conflicts, reduce diff churn.

The style chosen by reorder-python-imports has a single aim: reduce merge conflicts.

By having a single import per line, multiple contributors can add / remove imports from a single module without resulting in a conflict.

Consider the following example which causes a merge conflict:

# developer 1
-from typing import Dict, List
+from typing import Any, Dict, List

# developer 2
-from typing import Dict, List
+from typing import Dict, List, Tuple

no conflict with the style enforced by reorder-python-imports:

+from typing import Any
 from typing import Dict
 from typing import List
+from typing import Tuple

[gwax]

Worth noting that black guarantees that AST of input matches AST of output. Reordering imports would break this guarantee.

~Also, black plays along quite well with isort, so I'm not certain this is necessary.~

[digitalresistor]

@gwax I would like to point you to: https://github.com/ambv/black/issues/333#issuecomment-414123095 where some issues with isort are laid out...

[ghost]

Can we just get an option to disable checking imports? Otherwise my configured isort and black keep fighting with each other over which way is correct.

[tweakimp]

black means no options

[acdha]

@impala2 See @jensens’ https://gist.github.com/jensens/ce74e85bc297f3db46dad95fd08eb0c6 Black-compatible isort config. This is the example I come back to for why I'd like at least basic import sorting in Black: right now you have to coordinate two separate tools, one with non-trivial configuration, but with that one feature you could simply use Black with no other effort required.

[gforcada]

While reading this thread I came up with another idea...

Just like there is the -S switch that ignores string quotation marks, could there be another switch to ignore imports? This way, isort sorts the imports the way one likes, black takes care of the rest. :heart:

[tweakimp]

black means no options

[mec07]

@ambv Please also consider not just the ordering of the imports, but also:

• removing unused imports
• adding in imports that are getting used, e.g. if I type os.path.join(dir, filename) somewhere in the file but I haven't imported os, then just add the import os statement at the top of the file.

These two features are part of the Go formatting tool (which I run on save) and I find that they save me a ton of time when writing Go code. I sorely miss these features whenever I return to Python (I run black on save).

Thanks! 😍

[tweakimp]

Adding imports would change too much

[jaroel]

In the meanwhile I've kicked out isort as it turns out I don't give a shit about import ordering. Sorry for making a fuss. I'll take whatever black will do, big thank you up front!

[jgirardet]

* removing unused imports

You never know what could unused imports do (connect db ....) But the standard behavior could be to autoremove it, and the user should put it in nofmt block.

* adding in imports that are getting used, e.g. if I type `os.path.join(dir, filename)` somewhere in the file but I haven't imported `os`, then just add the `import os` statement at the top of the file.

import os or os.path ?

[mec07]

* removing unused imports

You never know what could unused imports do (connect db ....) But the standard behavior could be to autoremove it, and the user should put it in nofmt block.

I see what you're saying, but I think that is quite unusual, i.e. normally you would then use that db in the file.

* adding in imports that are getting used, e.g. if I type `os.path.join(dir, filename)` somewhere in the file but I haven't imported `os`, then just add the `import os` statement at the top of the file.

import os or os.path ?

Good idea, I like the suggestion, import os.path would work nicely in my example.

[asottile]

• adding in imports that are getting used, e.g. if I type os.path.join(dir, filename) somewhere in the file but I haven't imported os, then just add the import os statement at the top of the file.

import os or os.path ?

Good idea, I like the suggestion, import os.path would work nicely in my example.

this is impossible to know without actually performing the import (you would no longer be a static analysis tool):

# f.py
class D:
    x = 1

class C:
    d = D()

c = C()

# g.py
import f

print(f.c.d.x)  # don't `import f.c.d` it's very very wrong

[mec07]

That's a good point. Thanks for bringing that up. Sticking with import f would be the way to go.

On Fri, 8 Feb 2019, 13:12 Anthony Sottile <notifications@github.com wrote:

• adding in imports that are getting used, e.g. if I type os.path.join(dir, filename) somewhere in the file but I haven't imported os, then just add the import os statement at the top of the file.

import os or os.path ?

Good idea, I like the suggestion, import os.path would work nicely in my example.

this is impossible to know without actually performing the import (you would no longer be a static analysis tool):

f.pyclass D:

x = 1

class C: d = D()

c = C()

g.pyimport f

print(f.c.d.x) # don't import f.c.d it's very very wrong

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/ambv/black/issues/333#issuecomment-461798131, or mute the thread https://github.com/notifications/unsubscribe-auth/AG77vPg_thojrlfEyJmspNJVfm9o746Fks5vLXekgaJpZM4UhluF .

[asottile]

That's a good point. Thanks for bringing that up. Sticking with import f would be the way to go.

even that, you can't possibly know statically is correct either (taken from ImportSideEffects wiki page)

---

Importing a package does not mean that the sub-packages/sub-modules are imported (though sometimes it may look like that!)

Consider the following:

$ tree
.
└── pkg
    ├── __init__.py
    ├── mod2.py
    └── mod.py

1 directory, 3 files

$ tail -n 999 pkg/*
==> pkg/__init__.py <==

==> pkg/mod2.py <==
import pkg.mod
x = 2

==> pkg/mod.py <==
hi = 1

>>> import pkg
# Doesn't work!
>>> pkg.mod
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
AttributeError: 'module' object has no attribute 'mod'
# But if we import some module that has a side-effect of importing that module
>>> import pkg.mod2
# Suddenly it works!
# DON'T EVER DEPEND ON THIS BEHAVIOUR!
>>> pkg.mod
<module 'pkg.mod' from 'pkg/mod.py'>

[jgirardet]

SInce black is a numb formatter, it'll never know what the code is doing or not. can't guess it. So what it can do at the end ?

• format from bla import ble, bli, blo inline or explode one per line (already doing it)
• change verticale spacing : for example same root module not vertical space, different root module 1 new line separation.
• "maybe" put all stdlib first, in alphabetical order (but I 'm not sure if it will not break the ast).

I can't see much other things. Import is part of coding like declaring a new function. It's not necessarily only formatting. Nobody here ever disabled isort because of some import error ?

[digitalresistor]

To date on the projects I have worked on I have not disabled isort due to import errors. Although I would argue that if an import has side effects it is wrong. I don't want side effects when I import code.

[asottile]

(I don't use gevent, but my work does a lot), you frequently need to tell import reorderers off when dealing with gevent since it needs to be patched before importing other things

(I don't use matplotlib, but some of my work does) you frequently need to tell import reorderes off when using matplotlib as you need to select a rendering approach before importing other things

just some very common examples where import reordering requires configuration

[digitalresistor]

Yeah, not a fan of monkey patching in general...

[asottile]

Just so we're clear, I'm not advocating for monkeypatching :laughing: -- just pointing out that import reorderers tend to require some amount of configuration and

black means no options

[gwax]

As much as we may not like import side-effects, they do happen. Reordering can be dangerous. Adding / removing imports can be dangerous.

Encouraging nofmt for common scenarios suggests a design failure.

"black means no options" suggests not configuring.

Personally, I'd rather some effort go into making isort and black totally compatible and keep separate tools for separate tasks.

I find the no-AST changes guarantee of black quite comforting, especially when introducing it to large, existing projects.

[joaoe]

As mentioned, sorting top level imports can cause all sorts of issues and I personally would not feel comfortable with a tool like black doing that if my IDE can do that better, plus all the side effects when scripts setup paths before imports or when imports are wrapped in try/except blocks, etc...

However there is one thing that black could do which is to sort imported symbols in from .... import... statements which is completely benign and has no side-effects, e.g.

from bazinga import thats_my_spot, i_informed_you_thusly, rock_paper_scissors_lizard_spock

would become

from bazinga import (
    i_informed_you_thusly,
    rock_paper_scissors_lizard_spock,
    thats_my_spot,
)

[asottile]

@joaoe technically with __getattr__ (especially in py37+ where it is a first class construct for imports) even from import ordering can have side effects

[joaoe]

technically with __getattr__ (especially in py37+ where it is a first class construct for imports) even from import ordering can have side effects

That sounds like stuff you'd go full Linus Torvalds over the module developer.

[timothycrosley]

@ambv for what it's worth:

I agree with everything you said, especially in this comment here:

• In general, its implementation is problematic. Everything interesting happens in an __init__() method; it's based on string matching and not a syntax tree, and so on.

• Its configuration based on magic discovery of first-party/third-party code only works on activated virtualenvs, making it brittle for CI.

• It moves comments around in a way that is incompatible with Black.

• However, the killer thing for me is the example you give around imports placed after function calls being moved above the calls. Black would never do that because there are simply too many cases where this breaks the file. This makes code modding any larger codebase unsafe.

I am, with the help of other people in the community (especially @mkurnikov, thank you!) hoping to use these items as a roadmap for the next major isort release. I made isort many many years ago, and have been kind of blown away and surprised by the extent of its success, but I am not one to leave things to die after people start using them. I am happy to take responsibility or even just do my part to help make it better. I have learned a tremendous amount since I first released isort, and I believe it would match a lot more of what you expect if I were to build it today. But in the end, that's not what matters to me, as much as what can be done now that will have the best outcome for the Python community. If a better more robust and black compatible isort is that, which at this point I believe it is, I will do my best to ensure that happens. If instead, helping black do sorting was that, I would be happy to help with that effort as well. My intuition is that feels inconsistent with the other goals of Black, specifically at odds with not modifying AST. Re-ordering imports, even if done perfectly, does, inherently come with some danger of breaking previously functional code. That, of course, also implies more configuration being necessary than that used in gofmt - and what I believe Black is aiming for. Though certainly less than isort has now. My current goal is to soon release a black_isort that solves all of the above complaints, and ensures compatibility with black while exposing only the minimal necessary configuration options.

[ambv]

black_isort written by the original author of isort sounds like the ideal situation for us. Thanks so much for your input. Let me know if you need any help.

[asottile]

fwiw, both asottile/reorder_python_imports and sqlalchemyorg/zimports import sorters are black compatbile without configuration