rewrite of as imports changes the behavior of the imports

[keewis]

#1107 introduced a rewrite of import test.module as m styled imports to something like from test import module as m.

These changes are actually not equivalent (even though the name clash probably indicates poor design): using the standard import machinery, import test.module always tries to import the module test/module.py from somewhere on sys.path while from test import module will take anything it can get.

Consider a package like

$ tree test
test
├── __init__.py
└── module.py

0 directories, 2 files
$ cat test/__init__.py
def module():
    pass

with that, we can see the difference:

$ python -c 'import test.module as m; print(m)'
<module 'test.module' from '.../test/module.py'>
$ python -c 'from test import module as m; print(m)'
<function module at 0x7fb259fcf700>

which makes me a bit nervous since I usually don't look that deep into the code of dependencies.

Is there a setting to disable the rewrite?

[LouisTrezzini]

We're also seeing this issue on our code base.
We're looking forward to upgrade isort once we can control this behavior.

Thanks for the great rework!

[blisc]

I would find a setting to disable the rewrite very helpful.

[ppwwyyxx]

Related to this, the rewrite can be made a bit smarter: it currently rewrites import xx.yy as yy into from xx import yy as yy, instead of from xx import yy.
But I agree that it would be good to have an option to disable the rewrite (probably should be the default?) given that they are not equivalent.

[scop]

On Wed, 8 Jul 2020 at 12:15, Yuxin Wu ***@***.***> wrote: Related to this, the rewrite can be made a bit smarter: it currently rewrites import xx.yy as yy into from xx import yy as yy, instead of from xx import yy.

That's #1281

[terencehonles]

@keewis the example you gave is actually a bug (or at least should be considered a bug not a feature). I recently was made aware of Traps for the Unwary in Python’s Import System, but I personally discovered the issue you are presenting about a year ago or maybe more ago and I had to fix my code.

Given the tree you have:

mkdir test
touch test/module.py
echo 'def module(): pass' > test/__init__.py

While the examples you have do show that there's a name collision and you can pick one or the other by importing different ways, you're actually falling into The submodules are added to the package namespace trap (second example)

Using your tree you will see:

>>> import test
>>> test
<module 'test' from '.../test/__init__.py'>
>>> test.module
<function module at 0x7fabca5c8820>
>>> import test.module as module
>>> test.module
<module 'test.module' from '.../test/module.py'>

As you will notice, even though you performed the right import you have now shadowed test.module. Although you can properly access the module now you have lost the ability to access the variable, and you can see why this should be avoided. I'm not sure if you can give a scenario that wouldn't have this behavior, but at least for the simple case you provided I wanted to make sure you were aware that you were very likely doing something you really didn't want to be doing (and arguably it would be good if you were warned about this via another linting tool (I don't think I was) since I don't think this is necessarily isort's job)

[keewis]

For use cases: I can imagine someone using this to provide a computationally less expensive but also less useful version of a module.

I agree that this should be avoided as much as possible, and that's why I stated that this might indicate poor design. This issue is not so much that I want to use that behavior, but rather that I don't know (or usually can't control) what my dependencies might be doing. That's why I'd like to have a setting to control the rewrite.

All technical issues aside, isort has all sorts of settings to change the way it reformats imports depending on taste (with the goal of making that choice consistent within the project), so adding one for this should be reasonable.

[timothycrosley]

A fix for this has been deployed to PyPI in the 5.1.0 release,

Thanks everyone for reporting!

~Timothy