python / mypy

Optional static typing for Python
https://www.mypy-lang.org/
Other
18.18k stars 2.77k forks source link

Should __init__.py imports be treated as exports with --no-implicit-reexport behaviour? #10198

Open edaqa-uncountable opened 3 years ago

edaqa-uncountable commented 3 years ago

I'm uncertain whether --no-implicit-reexport has a "correct" behaviour when it comes to __init.py__ files.

If I have this __init__.py file:

from .mysub import MyName

My intent is to export MyName from the module. Currently this does not work however, and I need to do either:

from .mysub import MyName as MyName

or

from .mysub import MyName
__all__ = ['MyName']

Both of which are redundant.

I saw this issue on stub-generation that seems to imply that imports in an init (or otherwise unused imports) should be considered exports.

Shouldn't --no-implicit-reexport treat init files as doing exports for the module without the code having to use the redundant ...as... or __all__ form?

tucked commented 3 years ago

Bump

Projects (e.g. click and bidict) are starting to WONTFIX this saying they expect their users to set no_implicit_reexport = False. Some upstream input could definitely be helpful...

gaborbernat commented 3 years ago

I personally believe here mypy is in the right. And mypy should fail as it's doing now. However, the project might want to opt-in to an implicit re-export behavior if they don't want to use an explicit export instead. This setting should be done though by the project and not the downstream users though. Perhaps py.type file could contain some metadata related to this.

To put it in scope why mypy (rightly) complains here, consider the following example:

from typing import List

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

This module will provide both generate_list and List as available to import for the runtime. I think we can agree though that List should not be imported from this module, but rather keep importing it from typing. We need some way to explicitly state what's just used by the module, and what is meant to be imported by end-users.

Historically the __all__ was expressing this. Linters/IDEs used this to suggest imports as such. For __init__.py the from x import y as y was also used/worked.

jugmac00 commented 3 years ago

What is bad about using __all__?

gaborbernat commented 3 years ago

Some people find it odd that you have to:

In practice means making sure your __all__ list is up to date and correct is an error-prone job. I can live with these issues personally, but I understand the concern.

tucked commented 3 years ago

I wonder if it would make sense for mypy to detect something like

__all__ = list(locals())

as a way to disable no_implicit_reexport from the package side 🤔

davidism commented 3 years ago

Since Pallets ended up reintroducing this conversation, just wanted to chime in that after looking at the issue more we agree with @gaborbernat that mypy is right here, so we'll make bugfix releases with explicit exports in the form import name as name.

Either method is a bit noisy to me, so I think we could still brainstorm how this could be improved to make it easier / more natural to maintain.

srittau commented 3 years ago

I don't think __init__.py should be treated specially for two reasons:

gaborbernat commented 3 years ago

Either method is a bit noisy to me, so I think we could still brainstorm how this could be improved to make it easier / more natural to maintain.

I'd imagine we should enquire the python core developers about this 🤔 so probably someone should open a topic on discuss.python.org

edaqa-uncountable commented 3 years ago
  • have to define twice your names (once to import/define, once in __all__) and these definitions are not close to each other,

It's not limited to defining them twice, it's more than that. Consider I have a file abc.py in a module, and I use __all__ = ['One', 'Two']. Now I wish to export those public names from the module as a whole, so in my __init__.py file I have to list the names explicitly again.

I could live with having explicit exports be mentioned in __all__, but I think ti should be only once. There should be a way to re-export all those public names again.

The use of strings instead of symbols in __all__ is also an issue since it delays detection of defects.

edaqa-uncountable commented 3 years ago

At this point I'm kind of wishing we had an explicit export directive that could list individual names, rename, or all public symbols from a module. I guess that's why we need to get the Python code developers in the discussion.

Akuli commented 3 years ago

I wish star imports would re-export. Consider the following:

$ head main.py p/*
==> main.py <==
import p

def main() -> None:
    print(p.x + p.y)

==> p/a.py <==
x = 1

==> p/b.py <==
y = 2

==> p/__init__.py <==
from .a import *
from .b import *

$ mypy --strict main.py
main.py:4: error: Module has no attribute "x"
main.py:4: error: Module has no attribute "y"
Found 2 errors in 1 file (checked 1 source file)

While star imports are a bit controversial in general, I think re-exporting names is definitely a common pattern for them, and we use it a lot in typeshed. It would be nice if it would also work outside typeshed.

davidism commented 3 years ago

Thinking about how it's done in Pallets, if we ever use an import in the module, then it's not intended for export. The only time we want imports to be exported is if they're not used in the module, which happens to only be done in __init__ files.

Perhaps mypy could use this as a heuristic. We'd still be using flake8 to detect unused imports (or ignore them in __init__ files, so it wouldn't hide real issues with imports. If necessary, it could be behind an export_unused_import config.

flisboac commented 2 years ago

10826 is similar to this, ie. __all__ usage in __init__.py.

I think @erictraut's comment could be very useful in this context as well: https://github.com/python/mypy/issues/10826#issuecomment-1061418853

But summarizing, there is a proposal for the indentifiable __all__ idioms: https://github.com/python/typing/blob/master/docs/source/libraries.rst#library-interface