Missing __all__ in bidict/__init__.py leads to implicit reexport error with mypy in strict mode.

[FlorianKoegler]

Hi,
Pull request 107 has removed __all__ from bidict/__init__.py.

This leads to implicit reexports of all the imported classes etc., which generates a error when typechecking with mypy in strict mode (which sets --no-implicit-reexport).

So for example with test.py...

from bidict import bidict, BidirectionalMapping
element_by_symbol: BidirectionalMapping[str, str] = bidict({'H': 'hydrogen'})

... due to disallowing implicit reexports, the imports will not be found by mypy, while the code obviously works:

$ mypy --strict .\test.py
test.py:1: error: Module 'bidict' has no attribute 'bidict'
test.py:1: error: Module 'bidict' has no attribute 'BidirectionalMapping'; maybe "MutableBidirectionalMapping"?
Found 2 errors in 1 file (checked 1 source file)

Was there some reasoning behind removing __all__ or could it be re-added?
Thanks in advance. :)

[jab]

Hi @FlorianKoegler, thanks for opening this.

I deliberately removed __all__, deliberately do not configure mypy with --no-implicit-reexport, and want to keep it that way. From a maintainer's perspective, __all__ is error-prone and annoying to maintain, because it requires duplicating the names you mean to export in two different places. In return, it primarily just allows you to customize how wildcard imports work, which are bad practice in the first place. I prefer to not customize Python's default wildcard import behavior, where e.g. from bidict import * only imports the names in bidict.__init__ that don't begin with underscore. For anything that I import into bidict.__init__ that does not begin with underscore and that I don't want to export, I use as to give it a leading underscore, e.g.

bidict/bidict/__init__.py

Line 53 in 2f0a387

from sys import version_info as _version_info

I think mypy's --no-implicit-reexport is turned off by default for good reason. For many projects, including projects like bidict that are already being careful about what's exported from where, it's not a desired setting.

Hope this helps clarify!

[jab]

pallets/click#1880 (comment) seems applicable here:

This is noisy, I don't plan to make this change. You can add the following config to your project to allow Click (or other packages) to do this, it's a very common pattern. I suggest you have further discussion with mypy about the strictness of this, it sounds like they're open to it or may already have an open issue about it.

[mypy-click]
no_implicit_reexport = False

(substitute "bidict" in place of "click")

See also pallets/click#1879 (comment) (by one of the mypy developers):

That said this seems a bit too strict in this case, so maybe mypy should change its behavior to be more permissive here.

[gaborbernat]

I prefer to not customize Python's default wildcard import behavior, where, e.g. from bidict import *

Note here, though, that the __all__ is not only used for star import. I'd say that's probably one of the least interesting features provided by it. It makes it explicit what's to be exported from a module. Without this, if you have a module that uses typing.List that type is also exposed in that module namespace. For example:

from typing import List

def generate_list() -> List[int]:
    return [1]

This module will provides both generate_list and List as available to import. I think we can agree that List should not be imported from this module, but rather keep importing it from typing. Without the __all__ expressing this, the linters/IDEs have no way to know this.

[jab]

While I agree that the behavior without using __all__ has this problem, I don’t agree that __all__ is a good solution to the problem, for the reasons I gave above (manually maintaining the list of exports in a separate place from where they’re defined is tedious and error-prone). I’d rather wait for a better solution than use this bad solution.

[gaborbernat]

What would be a better solution in your eyes, would you propose any?

[jab]

I am hoping someone more qualified - who’s studied, or even designed, module systems for several languages - will propose something.

I know that there are other dynamic and gradually-typed languages whose designs avoided this problem from the start, but designing from scratch is a very different problem from improving an existing design.

In any case, if you’re interested in discussing Python module design questions further, I encourage you to do so on the Python-Dev list or some other place with a wider and more appropriate audience than this closed issue.

[gaborbernat]

While I agree that __all__ is not an ideal solution, is a working solution we have available today. Feels sub-optimal to leave this issue unresolved, and hope for some magical better solution for which there's no plan at all. It's not ideal but is the best we have, so until we come up with a better solution IMHO we should use it.

[jab]

If everyone always just accepted the one bad solution available, there would not be any pressure to create better solutions.

Please try not to presume one size fits all especially when there are tradeoffs involved and especially for open source projects where the work is done voluntarily. Open source maintainers have the right to evaluate the tradeoffs associated with any particular new suggestion and make the right decision for their projects based on their circumstances.

[jab]

I gave up and did the “import foo as foo” dance in the latest release, out now: https://bidict.readthedocs.io/en/v0.22.0/changelog.html

Hope this helps!