Skip to content

Remove all mention of 'module' from typeshed. #1156

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

Merged
merged 2 commits into from
Apr 13, 2017
Merged

Conversation

gvanrossum
Copy link
Member

Also add __dict__ to the existing types.ModuleType classes.

This depends on python/mypy#3107, and the tests may fail without that; also mypy's tests (even with that) won't fail with this. But I'd like to create this PR so @pkch can take a look. Also @matthiaskramm please note this, hopefully this won't break any pytype stuff (either in typeshed or at Google).

@matthiaskramm
Copy link
Contributor

Looks fine to me. One less keyword special to typeshed to worry about.

@@ -26,6 +26,7 @@ if sys.version_info >= (3, 4):
class ModuleType:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

__doc__ = ... # type: Optional[str]

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't be needed, since __doc__ is defined on object, which is implicitly inherited.

@pkch
Copy link
Contributor

pkch commented Apr 12, 2017

LGTM

@gvanrossum
Copy link
Member Author

D'oh, this seems a duplicate of #1127. But maybe we should make sure one isn't better than the other? There seem to be small differences.

@gvanrossum
Copy link
Member Author

Even the test failures (a bunch of internal errors in mypy) are similar.

@pkch
Copy link
Contributor

pkch commented Apr 12, 2017

Oh I think we should close mine in favor of yours; the only extra stuff I have is in werkzeug/, but I think I got it wrong (I shouldn't have removed it, they need more attributes).

@@ -102,15 +103,15 @@ chain = ... # type: type
double3prog = ... # type: type
endprogs = ... # type: Dict[str, Any]
pseudoprog = ... # type: type
re = ... # type: module
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can remove re = ... entirely.

Copy link
Member Author

@gvanrossum gvanrossum Apr 12, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Honestly a lot here should probably be removed -- it's only there because that's how pytype generates stubs (every imported thing ends up in the stub, even if it's clearly for purely internal reasons, and it never generates import statements). But I think that's a cleanup for a separate PR.

@gvanrossum
Copy link
Member Author

Once python/mypy#3107 is merged let's trigger tests here and then hopefully it will work.

Mostly to re-trigger tests
@JelleZijlstra JelleZijlstra merged commit 359c8cc into master Apr 13, 2017
@JelleZijlstra JelleZijlstra deleted the no-more-module branch April 13, 2017 15:40
@JelleZijlstra
Copy link
Member

Thanks!

gvanrossum added a commit to python/mypy that referenced this pull request Apr 13, 2017
Specifically to include python/typeshed#1156 which is important follow-up for #3107.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants