Repository navigation
Make stubgen re-export correctly imported names when module has no __all__ #3981
Description
Activity
Following up the discussion that came from #3169 (comment) and suggests (a), from checking a couple of simple examples (even the trivial examples in stubgen tests), I see that generated stubs will end up with no required imports. Most of the imports are normally not intended for reexport except on some high-level package modules (which are typically
__init__.pyfiles with just a lot of import statements), and the workaround for those cases will require the stubgen user to delete many import statements, and remove manyas namereexports in them too (and also figuring out which is which).My inclination towards (b) is that it's easier to add a few reexports when required.
Of course, any scenario can be fixed by adding an adequate
__all__to the source file, which makes user intent explicit and allows the tool to do the right thing in the first place instead of guessing.I also favor option (b). The rule about
import asin PEP 484 is about stub files, not implementation files, so I don't think it's relevant here.- Only (b) makes sense here.…On Fri, Sep 22, 2017 at 8:16 PM, Jelle Zijlstra ***@***.***> wrote: I also favor option (b). The rule about import as in PEP 484 is about stub files, not implementation files, so I don't think it's relevant here. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#3981 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ACwrMglMSLIlLPnDk26tGCbfxH4YRCD0ks5slHgrgaJpZM4PhDso> .-- --Guido van Rossum (python.org/~guido)
@JelleZijlstra
I am not sure what do you mean. Consider a (not so rare) situation:# lib/utils.py class PublicClass: ... # lib/__init__.py from .utils import PublicClass # main.py from lib import PublicClass
It works perfectly and passes mypy. But after running
stubgenonlibthis fails withModule 'lib' has no attribute 'PublicClass'because nowstubgenuses (b).Sure, lots of stubgen-generated code needs manual fixes later. I just think (b) is usually closer to being correct than (a) is. Perhaps we can make an additional exception for relative imports in files named
__init__.py, but that might introduce too much complexity.I just think (b) is usually closer to being correct than (a) is.
The point is that (a) just makes stubs a bit noisier (but they are not intended to be read often anyway), while (b) causes actual false positives, and what is worse, these false positives will be discovered not by a library maintainer or whoever produces stubs, but by users of mypy/typeshed.
- I wonder if there needs to be a special case for `__init__.py` when it imports things from submodules of the same package. I really don't want stubs containing `import os`, `import sys`, `import re` etc. (though I do see it would be easy to clean up).
I wonder if there needs to be a special case for
__init__.pywhen it imports things from submodules of the same package.I was thinking about two options:
a) Only re-export imported names in__init__.py
b) Always re-export names except names imported from standard library (the module names are currently found inmoduleinfo.py)I'm not sure what the right solution for this would be, but it'd be really helpful for the situation described below to:
- Add a new option for
no_implicit_reexportthat allows the option A mentioned above in @ilevkivskyi 's post - Or add a new configuration switch that makes everything imported (perhaps imported, but not used?) in an
__init__.pybe considered as part of an implicit__all__(which if I understood the code well enough, should also solve this problem).
# Code base has file structure like this. services/ -- first_service/ -- __init__.py -- some_stuff.py -- second_service/ -- __init__.py -- other_stuff.py# first_service/__init__.py from services.first_service.some_stuff import foo from services.first_service.some_stuff import other_cool_function
In our
__init__.py's for each of the services, we basically declare our interfaces for other services. I don't want to have to define an__all__for these, since it seems like I'd just be repeating myself in each of the__init__s just for this check (which I want to use since it's cool!).Not sure if anyone has any other work arounds or suggestions.
Thanks!
- Add a new option for
Here's an approach that I'm currently working on (these are the most important rules):
- If a module has
__all__(and we can determine its value), use it to decide what to re-export. Otherwise, follow the rules below. - Re-export all names imported from any submodule of
fooor_fooif the current module is a submodule offoo. (Submodules don't need to be direct submodules, andfoois a submodule offoo.) - Re-export all imported names that aren't used in the module.
I'm also thinking of having an option to disable (2).
In particular,
__init__is not special in the above rules.- If a module has
The motivation for the above rules is that I'd like stubgen to generate usable stubs with minimal manual work. When unsure about whether something should be part of a public API, we'd include it in the public API.
Now we'd have extra manual work to trim unnecessary exported names from the stubs, instead of adding missing names. Arguably removing names is less tedious than adding names, and also extra names are less of a problem than missing names, I'd say.
The motivation for the above rules is that I'd like stubgen to generate usable stubs with minimal manual work. When unsure about whether something should be part of a public API, we'd include it in the public API.
OK, this makes sense.
Thanks for the response. That makes sense to me and sounds better than my proposal. 😄
What is wrong with making
__init__.pya special case for re-exports? Is it not a common enough pattern that it makes clear what the module is exporting?stubgen has the
--export-lessflag that was added at some point
PEP-484 describes in which situations an imported name (which comes from an
import nameorfrom module import name) in a stub file are considered part of the exported interface. When generating a stub, stubgen must decide which of the imported names are intended to be reexported (ehich then must be added to the stub asimport name as name) and which aren't.Currently stubgen handles properly most situations where
__all__has been defined, but does not attempt to force reexports otherwise (some reexports may happen anyway if a name imported with an alias is required in an annotation).The main problem with the "no
__all__is set" is that we do not know author intent and we have to guess. There was some discussion within #3169 which was unresolved so I'm opening this issue as a followupThe 2 extreme alternatives when no
__all__are:a) Assume every name coming from an import in the source module is intended to be exported, and add the aliases in the stubs.
b) Assume no name coming from an import in the source module is intended to be exported, and just keep the imports needed for annotations (the current status)
Other more complicated heuristics may be possible in the spectrum from a to b.