Handle types.ModuleType

[pkch]

Fixes #1498

The only issue is that reveal_type reports '_importlib_modulespec.ModuleType' instead of 'typing.ModuleType' for module objects. I fix that in the next commit (I want to separate this PR into 2 commits, since my second commit adds a new feature to the parser, which I'm not sure is appropriate; this way it would be easy to just discard the 2nd commit).

Also, if this is merged, we can delete class module from builtins.pyi (it's not needed any more).

[JelleZijlstra]

types.pyi

[pkch]

just in time for the last commit =)

[pkch]

And the next commit is just to remove seemingly superfluous class module definitions in a few places.

[gvanrossum]

I also found that , in addition to removing module from builtins.pyi, we also need to add (at least) __dict__ to the ModuleType class in types.pyi. (And all this for PY2 and PY3.)

[gvanrossum]

Is it possible to prune the contents of this file some? It looks like you just copied from typeshed -- but in order to avoid slowing down the tests we should really only include attributes that tests reference.

[gvanrossum]

There are also other changes needed to typeshed; various stubs apparently reference module.

[gvanrossum]

Please add a \n character at the end of this line.

[gvanrossum]

I don't understand what happend to the tests, perhaps because you included a typeshed sync? I think you may have to undo that part.

[pkch]

@gvanrossum Ah, I didn't know I have to git add typeshed before git commit haha should work now!

Edit: nope, I'm looking into it.

[gvanrossum]

I debugged the test failure a little bit and found that this probably is needed, but not sufficient:

diff --git a/test-data/unit/fixtures/fine_grained.pyi b/test-data/unit/fixtures/fine_grained.pyi
index 5959df6..3a74177 100644
--- a/test-data/unit/fixtures/fine_grained.pyi
+++ b/test-data/unit/fixtures/fine_grained.pyi
@@ -19,6 +19,8 @@ class str:
 class float: pass
 class bytes: pass
 class tuple: pass
+class list: pass
 class function: pass
 class ellipsis: pass
-class module: pass
+import types  # So it exists
+

FWIW to debug this I modified errors.py to always enter pdb (changed line 542 into if True:). I ran pytest as follows to enter pdb for one of the failing tests:

pytest -s -n0 -k testTopLevelMissingModuleAttribute

Hope this helps, I probably won't have time for more assistance.

[gvanrossum]

Also, you cannot include the typeshed merge in this PR. You should only use commits on typeshed's master branch. AFAIK the typeshed PR is not required for this PR to pass (though it is required for some Dropbox internals to pass, but that's not important).

The order in which we commit things should be:

1. This PR, sans typeshed changes
2. The typeshed PR (Remove all mention of 'module' from typeshed. typeshed#1156)
3. Make a new mypy PR that syncs typeshed
4. Merge that

[pkch]

Tests: thanks, your analysis helped. Also, git bisect confirmed that tests started to fail (assuming merged into master) at 737434b from #2838.

They fail (apart from module --> types.ModuleType issue) because types uses type variable, which is not yet supported in #2838. I put in a temporary solution by slightly modifying server/deps.py; @JukkaL can you just overwrite it later with the proper implementation when you continue to work on #2838?

typeshed: yes, I agree, and I never merged the pending PR in python/typeshed#1156 into here (you're right, it's not necessary). All I did was update typeshed to the current commit used in mypy master (without that, AppVeyor refused to build with "unmergeable" error).

We should try to merge this as soon as possible, to save people from having code that breaks when module is repalced with types.ModuleType.

[ilevkivskyi]

@pkch Since fine grained incremental is still WIP, I think it is fine to merge this now. I am going to merge this when tests pass

[gvanrossum]

Changes of doing the same for builtins.function?

[ilevkivskyi]

Looking at types.FunctionType it seems to me that removing builtins.function will be quite simple.
@pkch Would you like to do this?

[pkch]

@ilevkivskyi Sure, but it seems that with this change, every test needs types.FunctionType in the test stubs (probably because builtins.function was used as a fallback). And that means builtins.pyi (and every custom bultins fixture) would have to import types. (On top of that, we'll also need to define list everywhere since types.pyi uses it.)

Do we want to do some workaround, like manually inject types.FunctionType into the test environment, without importing types? Not that types.pyi is huge, but it does have a generic in it.

[ilevkivskyi]

Do we want to do some workaround, like manually inject types.FunctionType into the test environment, without importing types? Not that types.pyi is huge, but it does have a generic in it.

You could just try adding import types, and see what is the performance impact for tests, if it is few percent, then it is OK, but a manual injection would be needed if there will be a significant slow-down in tests.