Skip to content

Conversation

@matthiaskramm
Copy link
Contributor

Modules can contain anything, so give them a generic __getattr__(name) -> Any.
This makes code like the following type-check, in pytype:

    sys.modules.get("random").randint

Modules can contain anything, so give them a generic "__getattr__(name) -> Any".
This makes code like the following type-check, in pytype:
    sys.modules.get("random").randint
@JelleZijlstra
Copy link
Member

Sounds reasonable, but this appears to break some of mypy's tests.

@matthiaskramm
Copy link
Contributor Author

Yep. I'll undo the Python 3 change for now.

@gvanrossum
Copy link
Member

Hold on, the test failure suggests that there's something fundamental wrong here. With this change mypy believes that every module has every attribute. E.g. import sys; sys.randint would pass (because the type of sys is ModuleType). That sure can't be right? I think it only failed with Python 3 because mypy's Python 2 tests are less comprehensive.

@gvanrossum
Copy link
Member

Maybe declaring sys.modules to be of type Dict[str, Any] would be a more appropriate solution? After all there's a pattern (I think used for lazy imports) where the module in sys.modules is replaced with a sort of proxy object that lazily loads attribute access.

(Instead of Dict[str, ModuleType])
Same as stdlib/3/.
@matthiaskramm
Copy link
Contributor Author

Fine with me. As a matter of fact, that's how it was already defined in stdlib/3/sys.pyi. So changing the Python 2 version, too.

@matthiaskramm matthiaskramm changed the title Add __getattr__ to ModuleType. Make sys.modules a Dict[str, Any] May 15, 2017
@matthiaskramm matthiaskramm changed the title Make sys.modules a Dict[str, Any] Make sys.modules a Dict[str, Any] May 15, 2017
Copy link
Member

@gvanrossum gvanrossum left a comment

Choose a reason for hiding this comment

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

Much better!

@JelleZijlstra JelleZijlstra merged commit 8d87085 into master May 20, 2017
@JelleZijlstra JelleZijlstra deleted the module branch May 20, 2017 18:08
@gvanrossum
Copy link
Member

[Hey @JelleZijlstra, nexttime, in addition to editing the commit subject line, could you also trim the local commits from the final commit message? In this case they are particularly unhelpful, since they were all undone.]

@JelleZijlstra
Copy link
Member

Makes sense, will remember that for next time.

JelleZijlstra added a commit that referenced this pull request May 21, 2017
matthiaskramm pushed a commit that referenced this pull request May 22, 2017
li-dan pushed a commit to li-dan/typeshed that referenced this pull request May 22, 2017
* Add __getattr__ to ModuleType.

Modules can contain anything, so give them a generic "__getattr__(name) -> Any".
This makes code like the following type-check, in pytype:
    sys.modules.get("random").randint

* undo Python 3 change

* Revert "undo Python 3 change"

This reverts commit 96cf2d5.

* Revert "Add __getattr__ to ModuleType."

This reverts commit 3ac1cf8.

* In stdlib/2/, make sys.modules a Dict[str, Any].

(Instead of Dict[str, ModuleType])
Same as stdlib/3/.
li-dan pushed a commit to li-dan/typeshed that referenced this pull request May 22, 2017
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