--no-implicit-reexport should treat names in __all__ as exported

[Zac-HD]

From HypothesisWorks/hypothesis#2017, passing --no-implicit-reexport with Mypy 0.710 causes error: Module 'hypothesis' has no attribute 'given' - except that it does, and given is listed in the __all__ attribute.

IMO when __all__ is present only those names should be considered explicitly re-exported, though taking the union of those and the from-import-x-as-x form would be reasonable too. Ignoring __all__ is pretty weird though!

[bluetech]

FWIW, I agree. I ran into this when enabling --no-implicit-reexport and importing from the sentry-sdk package (this is the file: https://github.com/getsentry/sentry-python/blob/0.9.2/sentry_sdk/__init__.py, cc @untitaker).

I also intuitively think __all__ should define the export list when present. [Personally, I found the "import as-rexports" rule surprising, so I'd go further and make __all__ the only way to reexport. But I realize it may be too heavy handed.]

no-implicit-reexport is a great feature, BTW.

[untitaker]

I didn't know about this issue. Is there a workaround we can add to the SDK?

[bluetech]

@untitaker Like @Zac-HD suggested in the Hypothesis issue, I just turned the feature off for sentry-sdk for now. I think we should wait to see whether the Mypy devs consider this a bug or not, before exploding the import * in sentry-sdk.

[JukkaL]

Agreed that this looks like a bug.

[JukkaL]

Anybody want to try to fix this? This doesn't sound very tricky.

[bluetech]

I started writing tests for the PR but there is a behavior I'm not sure about. Currently, star imports always reexport. That is, given

# private_module.py
a = 1
# public_module.py
from private_module import *

then from public_module import a is legal. This is considered an explicit reexport, so --no-implicit-reexport doesn't affect it.

This behavior is documented for stub files in PEP 484:

Modules and variables imported into the stub are not considered exported from the stub unless the import uses the import ... as ... form or the equivalent from ... import ... as ... form.

However, as an exception to the previous bullet, all objects imported into a stub using from ... import * are considered exported. (This makes it easier to re-export all objects from a given module that may vary by Python version.)

My question: should star-imports be considered explicit reexports also in regular (non-stub) files?

Pro: consistent with stub files, less boilerplate for intentional reexports.

Con: surprising, makes it impossible to use star imports in modules that want to control their exports.

My personal preference is to not make them explicit reexports, but don't feel strongly; will implement which ever way is decided.

[Zac-HD]

My personal preference is to not make them explicit reexports, but don't feel strongly; will implement which ever way is decided.

I share your preference, though I would say it's strong for me. Brevity is a virtue for stub files, but for actual source code "explicit is better than implicit" - if something from a star-import is meant to be reexported, it can be added to __all__ 😄

[msullivan]

My initial inclination was that they need to continue to be considered explicit, since it is a (perhaps unfortunate) common pattern to reexport everything.

But thinking about it some more, now that mypy supports in-file configuration directives, it will be easy to disable --no-implicit-reexport for just a module that is doing this.

So yeah, let's make it not considered an explicit reexport.