Stricter handling of submodules as attributes

[ilevkivskyi]

Fixes #20174

The idea is quite straightforward: we only allow foo.bar without explicit re-export if foo.bar was imported in any transitive dependency (and not in some unrelated module). Note: only import foo.bar takes effect, effect of using from foo.bar import ... is not propagated for two reasons:

• It will cost large performance penalty
• It is relatively obscure Python feature that may be considered by some as "implementation detail"

[sterliakov]

Random check: primer installs types-pyOpenSSL for mitmproxy, but it should not (pyOpenSSL is typed, its stubs are no longer updated for a long time, minimal pyOpenSSL version supported by mitmproxy ships py.typed), and does not install pyOpenSSL itself.

The stubs have empty __init__.pyi, so the error is correct according to them. Real pyOpenSSL, however, has from OpenSSL import crypto in __init__.py, so the error does not happen with real installation. I tried a local run with mypy from this PR and get no such errors with pyOpenSSL installed, but can reproduce the primer output after installing deprecated types-pyOpenSSL instead.

[sterliakov]

(should we clean up type dependencies in mypy-primer?..)

[sterliakov]

A few other entries:

• beartype: I'm lost there, sorry
• argclinic: looks correct to me
• zulip: correct, checked manually in Django shell (import django does not load django.core.serializers)
• mitmproxy: correct, see above
• cki-lib: correct, boto3-stubs do not import urllib.parse. At runtime import boto3, urllib makes urllib.parse available, but there are no matches of urllib\.parse in boto3-stubs (with some extras I have installed in some venv). Primer does not install those anyway, so import botocore and import boto3 are unresolved, urllib.parse never becomes visible there.
• ibis: weird. ibis.backends.snowflake should be available when checking/running ibis.backends.snowflake.tests.conftest, right? It's a parent package, it doesn't need deeper dotted imports, we are already inside...
• pandera: Wrong? pandas.__init__ > pandas.core.api.__init__ > pandas.core.dtypes.dtypes should import pandas.core.dtypes. Are you handling the case where only submodule brings the module into scope?
• scipy-stubs: correct and expected, they want mypy to report that, according to the comment https://github.com/scipy/scipy-stubs/blob/master/tests/misc/test_subpackage_export.pyi

Testcase for a pandera error:

[case testIncrementalAccessSubmoduleWithoutExplicitImportNested]
import pandas

pandas.core.dtypes

[file pandas/__init__.py]
import pandas.core.api

[file pandas/core/__init__.py]
[file pandas/core/api.py]
from pandas.core.dtypes.dtypes import X

[file pandas/core/dtypes/__init__.py]
[file pandas/core/dtypes/dtypes.py]
X = 0
[out]
[out2]

(import from seems to be the crux, replacing it with import pandas.core.dtypes.dtypes removes the error)

[ilevkivskyi]

@sterliakov Thanks for the analysis! I think both ibis and pandera can be handled by adding ancestors.

[sterliakov]

Nice! That was correct, and you won't have to untangle the beartype metaprogramming...

[ilevkivskyi]

Oh, but then something like this fails on the second run

[case testIncrementalAccessSubmoduleWithoutExplicitImportNested]
import a

[file a.py]
import pandas
pandas.core.dtypes

[file a.py.2]
import pandas
pandas.core.dtypes
# touch

[file pandas/__init__.py]
import pandas.core.api

[file pandas/core/__init__.py]
[file pandas/core/api.py]
from pandas.core.dtypes.dtypes import X

[file pandas/core/dtypes/__init__.py]
[file pandas/core/dtypes/dtypes.py]
X = 0
[out]
[out2]

Because ancestors are not treated as a dependency, so they may not be loaded. Not sure what to do here, we can't really make ancestors dependencies because this will create a lot of extra "import cycles". So maybe we will need to accept few false positives for the sake of consistency between cold an warm runs.

[JelleZijlstra]

(should we clean up type dependencies in mypy-primer?..)

Yes, you can make a PR to github.com/hauntsaninja/mypy-primer to fix this.

[ilevkivskyi]

So maybe we will need to accept few false positives for the sake of consistency between cold an warm runs.

Or maybe we can load the ancestors if they are fresh. Need some time to figure out if this will always be correct.

[sterliakov]

Ough.

Not sure what to do here, we can't really make ancestors dependencies because this will create a lot of extra "import cycles".

If the ancestor module is fresh, you should be able to add it as a dependency "for free", right? And if it's stale, it should already be analyzed as another target, so it will be in the modules map anyway. Am I missing something?

[ilevkivskyi]

If the ancestor module is fresh, you should be able to add it as a dependency "for free", right? And if it's stale, it should already be analyzed as another target, so it will be in the modules map anyway. Am I missing something?

Unfortunately, this doesn't work. For example, in test case I posted, if I touch pandas/core/dtypes/__init__.py in the second run as well, then technically it may be analysed in arbitrary order, since currently no other module depends on it.

The real problem is the difference between how we handle

import foo.bar.baz

vs

from foo.bar import baz

Although runtime effect on foo of both is ~identical (bar is added to foo globals), we do not add dependency on foo after the from import.

So the only robust solution is to handle from import in the same way as we handle plain import. But, this is a big change with bad performance implications (I observed up to 10% slowdown in some examples) for a rare edge case.

After thinking more about this, it is quite natural to rely on the fact that after import foo.bar.baz, foo.bar is a valid expression (since this is the only way such import could work at runtime, import will only add foo to module gloabals). On the contrary, relying on the fact that after from foo.bar import baz you can write import foo; foo.bar in other module is quite weird. TBH I am not even sure why Python adds bar to foo at runtime, the only thing needed for the import to work is to add baz to the importer globals.

So my conclusion is that we can declare the runtime behaviour of from import an "implementation detail", and don't support this. After all, there are two simple fixes, either add an explicit import, or an explicit re-export.

[JukkaL]

What about adding a fast path, where we first check if the target is in the direct import dependencies of the current module? For example, if current module has import foo.bar, then foo.bar will be a direct dep. Now we can immediately see that foo.bar will be visible due to it being in import_map, without having to do the recursive search?

[github-actions]

Diff from mypy_primer, showing the effect of this PR on open source code:

beartype (https://github.com/beartype/beartype)
+ beartype/_data/typing/datatyping.py:671: error: Name "beartype._check.forward.reference.fwdrefabc.BeartypeForwardRefABC" is not defined  [name-defined]
+ beartype/_check/forward/reference/fwdrefmeta.py:61: error: Unused "type: ignore" comment  [unused-ignore]
+ beartype/_check/forward/reference/fwdrefmeta.py:270: error: Unused "type: ignore" comment  [unused-ignore]
+ beartype/_check/forward/reference/fwdrefmeta.py:298: error: Unused "type: ignore" comment  [unused-ignore]
+ beartype/_check/forward/reference/fwdrefmeta.py:327: error: Unused "type: ignore" comment  [unused-ignore]
+ beartype/_check/forward/reference/fwdrefmeta.py:380: error: Unused "type: ignore" comment  [unused-ignore]
+ beartype/_check/forward/reference/fwdrefabc.py:218: error: Unused "type: ignore" comment  [unused-ignore]
+ beartype/_check/forward/reference/fwdrefmake.py:91: error: Unused "type: ignore" comment  [unused-ignore]

CPython (Argument Clinic) (https://github.com/python/cpython)
+ Tools/clinic/libclinic/clanguage.py:67: error: Module has no attribute "cpp"  [attr-defined]

zulip (https://github.com/zulip/zulip)
+ zilencer/migrations/0026_auditlog_models_extra_data_json.py:17: error: Module has no attribute "serializers"  [attr-defined]
+ zilencer/migrations/0026_auditlog_models_extra_data_json.py:24: error: Module has no attribute "serializers"  [attr-defined]
+ zerver/migrations/0452_realmauditlog_extra_data_json.py:17: error: Module has no attribute "serializers"  [attr-defined]

cki-lib (https://gitlab.com/cki-project/cki-lib)
- cki_lib/s3bucket.py:76: error: Incompatible types in assignment (expression has type "ParseResult", variable has type "str")  [assignment]
+ cki_lib/s3bucket.py:76: error: Module has no attribute "parse"  [attr-defined]

mitmproxy (https://github.com/mitmproxy/mitmproxy)
+ mitmproxy/certs.py:98: error: Name "OpenSSL.crypto.X509" is not defined  [name-defined]
+ mitmproxy/certs.py:102: error: Name "OpenSSL.crypto.X509" is not defined  [name-defined]
+ mitmproxy/certs.py:103: error: Module has no attribute "crypto"  [attr-defined]

ibis (https://github.com/ibis-project/ibis)
+ ibis/backends/snowflake/tests/conftest.py:212: error: Module has no attribute "snowflake"  [attr-defined]

pandera (https://github.com/pandera-dev/pandera)
+ pandera/engines/type_aliases.py:10: error: Module has no attribute "dtypes"  [attr-defined]
+ pandera/engines/type_aliases.py:11: error: Name "pd.core.dtypes.base.ExtensionDtype" is not defined  [name-defined]
+ pandera/api/pandas/types.py:18: error: Name "pd.core.dtypes.base.ExtensionDtype" is not defined  [name-defined]
+ pandera/typing/pandas.py:106: error: Name "pd.core.dtypes.base.ExtensionDtype" is not defined  [name-defined]
- pandera/api/pandas/components.py:23: error: Incompatible default for argument "dtype" (default has type "None", argument has type "str | type | DataType | ExtensionDtype | dtype[Any]")  [assignment]
- pandera/api/pandas/components.py:23: note: PEP 484 prohibits implicit Optional. Accordingly, mypy has changed its default to no_implicit_optional=True
- pandera/api/pandas/components.py:23: note: Use https://github.com/hauntsaninja/no_implicit_optional to automatically upgrade your codebase
- pandera/api/pandas/components.py:365: error: Argument "dtype" to "Column" has incompatible type "DataType | None"; expected "str | type | DataType | ExtensionDtype | dtype[Any]"  [arg-type]
+ tests/pandas/test_typing.py:320: error: Name "pd.core.dtypes.base.ExtensionDtype" is not defined  [name-defined]
+ tests/pandas/test_typing.py:482: error: Name "pd.core.dtypes.base.ExtensionDtype" is not defined  [name-defined]
+ tests/pandas/test_schema_components.py:663: error: Unused "type: ignore" comment  [unused-ignore]
- tests/pandas/test_multithreaded.py:11: error: Type argument "floating[_32Bit]" of "Series" must be a subtype of "bool | int | str | float | Bool | <30 more items>"  [type-var]
- tests/pandas/test_model.py:193: error: Type argument "list[int]" of "Series" must be a subtype of "bool | int | str | float | Bool | <30 more items>"  [type-var]
+ tests/pandas/test_model.py:178: error: Unused "type: ignore" comment  [unused-ignore]

scipy-stubs (https://github.com/scipy/scipy-stubs)
+ tests/misc/test_subpackage_export.pyi:7: error: Module has no attribute "misc"  [attr-defined]
+ tests/misc/test_subpackage_empty.pyi:17: error: Module has no attribute "common"  [attr-defined]
+ tests/misc/test_subpackage_empty.pyi:18: error: Module has no attribute "doccer"  [attr-defined]