false positive 'useless-import-alias' error for mypy-compatible explicit re-exports

[jab]

Bug description

Suppose a package has the following layout:

package/
  _submodule1.py  # defines Api1
  _submodule2.py  # defines Api2
  __init__.py     # imports and re-exports Api1 and Api2

Since the submodules here implement public APIs, __init__.py imports and re-exports them, expecting users to import them from the public, top-level package, e.g. from package import Api1.

Since the implementations of Api1 and Api2 are complex, they are split into _submodule1.py and _submodule2.py for better maintainability and separation of concerns.

So __init__.py looks like this:

from ._submodule1 import Api1 as Api1
from ._submodule2 import APi2 as Api2

The reason for the as aliases here is to be explicit that these imports are for the purpose of re-export (without having to resort to defining __all__, which is error-prone). Without the as aliases, popular linters such as mypy will raise an "implicit re-export" error (docs -- part of mypy --strict).

However, pylint does not currently understand this usage, and raises "useless-import-alias" errors.

Example real-world code triggering pylint false positive errors: https://github.com/jab/bidict/blob/caf703e959ed4471bc391a7794411864c1d6ab9d/bidict/__init__.py#L61-L78

Configuration

No response

Command used

pylint

Pylint output

************* Module bidict
bidict/__init__.py:61:0: C0414: Import alias does not rename original package (useless-import-alias)
bidict/__init__.py:61:0: C0414: Import alias does not rename original package (useless-import-alias)
bidict/__init__.py:62:0: C0414: Import alias does not rename original package (useless-import-alias)
bidict/__init__.py:62:0: C0414: Import alias does not rename original package (useless-import-alias)
bidict/__init__.py:62:0: C0414: Import alias does not rename original package (useless-import-alias)
bidict/__init__.py:63:0: C0414: Import alias does not rename original package (useless-import-alias)
bidict/__init__.py:63:0: C0414: Import alias does not rename original package (useless-import-alias)
bidict/__init__.py:64:0: C0414: Import alias does not rename original package (useless-import-alias)
bidict/__init__.py:65:0: C0414: Import alias does not rename original package (useless-import-alias)
bidict/__init__.py:66:0: C0414: Import alias does not rename original package (useless-import-alias)
bidict/__init__.py:66:0: C0414: Import alias does not rename original package (useless-import-alias)
bidict/__init__.py:67:0: C0414: Import alias does not rename original package (useless-import-alias)
bidict/__init__.py:68:0: C0414: Import alias does not rename original package (useless-import-alias)
bidict/__init__.py:69:0: C0414: Import alias does not rename original package (useless-import-alias)
bidict/__init__.py:69:0: C0414: Import alias does not rename original package (useless-import-alias)
bidict/__init__.py:69:0: C0414: Import alias does not rename original package (useless-import-alias)
bidict/__init__.py:70:0: C0414: Import alias does not rename original package (useless-import-alias)
bidict/__init__.py:70:0: C0414: Import alias does not rename original package (useless-import-alias)
bidict/__init__.py:70:0: C0414: Import alias does not rename original package (useless-import-alias)
bidict/__init__.py:70:0: C0414: Import alias does not rename original package (useless-import-alias)
bidict/__init__.py:70:0: C0414: Import alias does not rename original package (useless-import-alias)
bidict/__init__.py:71:0: C0414: Import alias does not rename original package (useless-import-alias)
bidict/__init__.py:71:0: C0414: Import alias does not rename original package (useless-import-alias)
bidict/__init__.py:72:0: C0414: Import alias does not rename original package (useless-import-alias)
bidict/__init__.py:72:0: C0414: Import alias does not rename original package (useless-import-alias)
bidict/__init__.py:72:0: C0414: Import alias does not rename original package (useless-import-alias)
bidict/__init__.py:73:0: C0414: Import alias does not rename original package (useless-import-alias)
bidict/__init__.py:74:0: C0414: Import alias does not rename original package (useless-import-alias)
bidict/__init__.py:74:0: C0414: Import alias does not rename original package (useless-import-alias)
bidict/__init__.py:74:0: C0414: Import alias does not rename original package (useless-import-alias)
bidict/__init__.py:74:0: C0414: Import alias does not rename original package (useless-import-alias)
bidict/__init__.py:74:0: C0414: Import alias does not rename original package (useless-import-alias)
bidict/__init__.py:74:0: C0414: Import alias does not rename original package (useless-import-alias)
bidict/__init__.py:74:0: C0414: Import alias does not rename original package (useless-import-alias)
bidict/__init__.py:74:0: C0414: Import alias does not rename original package (useless-import-alias)
bidict/__init__.py:74:0: C0414: Import alias does not rename original package (useless-import-alias)
bidict/__init__.py:74:0: C0414: Import alias does not rename original package (useless-import-alias)
bidict/__init__.py:74:0: C0414: Import alias does not rename original package (useless-import-alias)

Expected behavior

No "useless-import-alias" errors should be flagged.

Pylint version

pylint 2.13.2
astroid 2.11.2
Python 3.10.2 (main, Feb  2 2022, 07:36:01) [Clang 12.0.0 (clang-1200.0.32.29)]

OS / Environment

No response

Additional dependencies

No response

[Pierre-Sassoulas]

The reason for the as aliases here is to be explicit that these imports are for the purpose of re-export (without having to resort to defining all, which is error-prone).

I think __all__is the way to be explicit about the API of a module.That way you have the API documented in one place at the top of the module without having to check what exactly is imported with import x as x. I never heard about import x as x and never did the implementer of the check, but I saw the mypy documentation you linked, let's see how widely used this is.

[cdce8p]

I don't think there is a way for pylint to detect if the reexport is intended or not. Maybe we could ignore __init__.py files 🤔 However, that might be unexpected to the general user.

Probably the easiest solution in your case would be to add a module level pylint: disable=useless-import-alias (before any imports).

[jab]

Yeah, other linters like mypy specifically support as rather than just __all__ for this, since so many people have been burned by using __all__.

__all__ requires maintaining exports as a list of strings, which are all-too-easy to typo (and often tools and IDEs can’t detect when this happens), and also separately (and often far away) from where they’re imported / defined, which is also fragile and error-prone.

[Pierre-Sassoulas]

tools and IDEs can’t detect when this happens

Yeah I remember when I used liclipse (eclipse + pydev) this was a pain. This is an issue with the IDE though, Pycharm Community Edition handle this correctly.

[jab]

Sure, some IDEs can help with typos in __all__, but that's only one part of the problem with __all__. More problematic is that it forces you to maintain exports separately from where they're imported / defined, which makes it too easy for __all__ to drift out of sync as changes are made to the intended exports.

[Pierre-Sassoulas]

As @cdce8p said, the solution is to disable. I think this is going to stay that way because I don't see how pylint can guess the intent of the implementer. We could make this check optional but I think if you're making library API using this you're more able to disable the check than a beginner making a genuine mistake is to activate it. We're going to document this in the useless-import-alias documentation.

[jab]

Ok, thanks for that. As usage of mypy and mypy-style explicit re-imports continues to grow, it would be interesting to know how many pylint users end up having to disable useless-import-alias, and whether that amount ever crosses some threshold for being better as an opt-in rather than an opt-out. Not sure how much usage data you collect though for such decisions (e.g. by looking at usage from open source codebases).

[Pierre-Sassoulas]

crosses some threshold for being better as an opt-in rather than an opt-out

An alternative solution would be to not raise this message in __init__.py.

Not sure how much usage data you collect though for such decisions (e.g. by looking at usage from open source codebases).

To be frank, it's mostly opened issues and the thumbs-up / comments those issues gather. I'm not looking specifically at open sources projects for each messages it's really time consuming. A well researched comments on this issue with stats and sources, a proposition that is easily implementable with a better result than what we have currently, or this issue gathering 50+ thumbs up and a lot of attention would definitely make us reconsider.

[jab]

Got it, good to know.

[jab]

Just noticed https://github.com/microsoft/pyright/releases/tag/1.1.278

Changed the reportUnusedImport check to not report an error for "from y import x as x" since x is considered to be re-exported in this case. Previously, this case was exempted only for type stubs.

One more tool (pyright) flipping in this direction, fwiw.

[Pierre-Sassoulas]

We'll need an option to exclude __init__ for the check if this become widespread. Reopening in order to not duplicate info.