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

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

Open
edaqa-uncountable opened this issue Mar 11, 2021 · 13 comments
Labels
feature topic-implicit-reexport The --no-implicit-reexport option

Comments

@edaqa-uncountable
Copy link

edaqa-uncountable commented Mar 11, 2021

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
Copy link

tucked commented May 12, 2021

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
Copy link
Contributor

gaborbernat commented May 12, 2021

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
Copy link
Contributor

What is bad about using __all__?

@gaborbernat
Copy link
Contributor

Some people find it odd that you have to:

  • have to define twice your names (once to import/define, once in __all__) and these definitions are not close to each other,
  • the definition entries within __all__ is a string (important because typos are not caught early because of this)

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
Copy link

tucked commented May 14, 2021

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
Copy link

davidism commented May 14, 2021

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
Copy link
Contributor

srittau commented May 14, 2021

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

  • It's inconsistent and inconsistency is confusing.
  • Often, __init__.py is not just a collection of re-exports, but contains the bulk of the implementation, with a few utilities or special cases in submodules. In this case, re-exports are as problematic as in any other file.
  • Edit: A third reason is that foo.py and foo/__init__.py should work the same.

@gaborbernat
Copy link
Contributor

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
Copy link
Author

  • 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
Copy link
Author

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
Copy link
Contributor

Akuli commented Jun 6, 2021

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
Copy link

davidism commented Jun 6, 2021

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
Copy link

flisboac commented Mar 8, 2022

#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: #10826 (comment)

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature topic-implicit-reexport The --no-implicit-reexport option
Projects
None yet
Development

No branches or pull requests

9 participants