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 all (*) from modules better be positioned before explicit local imports #1504

Closed
besfahbod opened this issue Sep 28, 2020 · 3 comments
Labels
enhancement New feature or request help wanted Extra attention is needed idea_for_next_major_release
Milestone

Comments

@besfahbod
Copy link

besfahbod commented Sep 28, 2020

In general, it's recommended to not use "import all" pattern, or at least not without explicit __all__ defined.

However, many code-bases still do so, for good reasons, such as reducing friction in adding new items to a shared library.

Now, assuming we're okay with import * in general, and specifically from _<dir-name> import * forms, the example below demonstrates a problem when having import * statement at the end of the local-imports list, instead of first.


Here's the structure of the package bar, which has an internal module _foo, as well as module _bar that's considered the "main" module of the package.

The file contents are like:

bar/__init__.py:

from ._foo import Any, All, Not # ...
from ._bar import *

# [...]

bar/_bar.py:

from typing import Any, List # ...

# [...]

bar/_foo.py:

class Any(...):
    # [...]

class All(...):
    # [...]

class Not(...):
    # [...]

# [...]

In this set up, the class Any from typing is overshadowing the class Any defined in bar/_foo.py, and re-export explicitly as part of the "public API" of package bar.

If we change keep import * statements at the beginning of local-imports block, we would prevent such implicit overrides from external imports, reducing friction for developers.

So, isort would modify bar/__init__.py to look like:

from ._bar import *
from ._foo import Any, All, Not # ...

# [...]

I don't know if this has been discussed before (really hard to search for terms "import" and "all" in this repo), but like to suggest updating isort to change the behavior here.

What do you think?

@timothycrosley timothycrosley modified the milestones: 5.6.0, 5.7.0 Sep 29, 2020
@timothycrosley timothycrosley added enhancement New feature or request help wanted Extra attention is needed labels Sep 29, 2020
@timothycrosley
Copy link
Member

@besfahbod I think this is a great idea! This will likely surprise many who encounter, as all other non strictly alphabetical sorting behaviour does, but I think that can be solved with some documentation updates. Overall, I think this the exact kind of refinement that I'd love to include in isort now that it is a more mature tool.

Thanks!

~Timothy

@besfahbod
Copy link
Author

besfahbod commented Sep 29, 2020

Thanks, @timothycrosley! Agreed.

Not sure if most people agree or not, but something I've been doing personally has been to even keep import * statements in a separate block from explicit-import statements. If we do that, we can keep maintaining the promise of "always alphabetical order per block". #mytwocents

@timothycrosley timothycrosley modified the milestones: 5.7.0, 5.8.0 Dec 31, 2020
@timothycrosley
Copy link
Member

This has been implemented in develop (using the new --star-first option) with plans to make default in next major release.

Thanks!

~Timothy

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed idea_for_next_major_release
Projects
None yet
Development

No branches or pull requests

2 participants