Automatic nested duplicate import removal

[timothycrosley]

Currently, isort only removes duplicate imports if they are in the same import sorting block:

import a
import a
import b
import b

Becomes:

import a
import b

However, if imports are declared globally but then reimported later, isort leaves them unchanced:

import a
import b


def my_function():
     import a
     import b
     do_things()

Ideally, isort would remove both imports within the function, as they are already declared globally. This function should live under a configuration flag, such as remove_reimports_of_global_imports that defaults to True. Note that while nested imports that are identical to global imports should be removed, no separate groups of nested imports should effect each other or effect global imports:

def function_a():
   import os


def function_b():
   import os


import os

Should remain unchanged.

[razinc]

Hello @timothycrosley,

I'm interested to look into this issue but I'm afraid this is too advance for me. Can you please help to redirect me to which file I should look at to assess this?

Thanks.

[anirudnits]

in addition to nested redundant imports, isort also doesn't remove duplicated imports if they are not in the same import block. For example:

import a
import b

print("hello all")

import b

This would remain unchanged. I guess we can also include this use-case under the purview of this issue.

[anirudnits]

@razinc the config flags are defined here: https://github.com/PyCQA/isort/blob/5f6bc03f2000aec3e70b65324d940ce6f1395d25/isort/settings.py#L115 You can get the parsed file contents here: https://github.com/PyCQA/isort/blob/develop/isort/parse.py

Just off the top of my head, I guess you will need to introduce some kind of persistence in https://github.com/PyCQA/isort/blob/5f6bc03f2000aec3e70b65324d940ce6f1395d25/isort/core.py#L28 or somewhere else, to compare between imports in different import blocks.

[razinc]

Hi @anirudnits,

Please allow me to ask a basic question. I forked & clone this repo into my machine. May I know how I can use isort locally in my machine? For example, I made some edits to the files & I want to see how it works on my dummy files. This will help me to understand the flow better.

Thanks.

[anirudnits]

@razinc happy to help :) Contributing guidelines are here: https://github.com/PyCQA/isort/blob/develop/docs/contributing/1.-contributing-guide.md. You can write tests and build/run the modified code to see if it has the intended behavior or not. Does this answer your question?

[razinc]

Hi @anirudnits,

Yes, this answers my question & should get me started. Let me try it first.

Thanks.

[razinc]

Hi @anirudnits,

I'm using local development for my test case. Step 1 to 4 runs smoothly. I have some confusions as stated below:

1. Is this what it means by ./scripts/test.sh should yield Success: no issues found (step 5): I'm confused because I can't see "Success: no issues found"

2. Is this what it means by ./scripts/clean.sh should yield a Safety report checking packages (step 6):

I don't expect any problem with these commands as I don't make any changes.

Additional info: OS: Fedora 32 (Workstation Edition) Python version: 3.8.5 Poetry version: 1.1.2

Thanks.

[anirudnits]

@razinc everything looks cool. I personally use the docker environment to build/run isort but as you haven't really changed anything yet, it should be fine.

[razinc]

Hi,

If it's okay, I need more time on this.

Thanks.

[anirudnits]

@razinc any updates?

[razinc]

Hi @anirudnits,

I noticed the import section resets when going into the function due to this line. I tried to disable it but it totally messed up the output. I'm still investigating how to do it properly.

Thanks.

[LeetaoGoooo]

Is anyone else working on this problem? I've looked at the code and it seems to be just a few spaces not handled properly causing the problem

[doublethefish]

This is a linting issue and isort shouldn't fix this automatically, isort is doing the right thing and this issue should be closed. Let me explain:

Same-scope ordering is (pretty much) idempotent, but duplicate imports in different scopes/blocks is a code error that tools cannot reason about and, therefore, linters detect and then the developer should fix, being the only person who can make the best choice.

Known intent & stable collapse

Because the following import behaviour is known, stable and deterministic, we can infer what the user intended here, with a very high degree of confidence. Thus we can reorder and collapse with no side effects and the behaviour is idempotent.

NB: it wouldn't be idempotent if import b was the first import, but we're running a "sort" ;)

import a
import a
import b
import a
import b
import b

# So we get:

import a
import b

Unknown developer intent (unsafe to collapse/combine)

In the second case, using different scopes, we can't infer the intent so we shouldn't change the code automagically. For example, the developer might have originally intended the import to be at inner scope to avoid cyclic import bugs (hard to detect), we just don't know.

Because we can't reason about the original intent, the fix should be left to the developer to reason through.

import a
import b


def my_function():
     import a  # fails with pylint `reimported` & `redefined-outer-name`
     import b. # also fails with pylint `reimported` & `redefined-outer-name`
     do_things()

Possible aliasing side effect (also unsafe to collapse/combine)

Although it looks obvious what the following code does, if print has been monkey patched, or if other global-state modifying code (aka aliasing) is there we can't reason about what actually happens and therefore can't automagically fix it. Therefore, again, it is down to the linters to detect the issue and the developer to do the right thing.

import a
import b

print("hello all")

import b. # fails with pylint `reimported` & `wrong-import-position`