Support module-level __getattr__#3647
Conversation
|
I think the Travis failure is a flake. |
|
The Travis flakiness is rather annoying. I think I will get in touch with them and see if they have any ideas of why it is doing that. As for the PR, I left a couple of comments, I think the only big question is the scope of support for this, how limited/free we want to make that. I think the PEP might need some clarification either way. |
test-data/unit/check-modules.test
Outdated
| reveal_type(has_getattr.any_attribute) # E: Revealed type is 'Any' | ||
|
|
||
| [file has_getattr.py] | ||
| __getattr__ = 3 |
There was a problem hiding this comment.
I don't think we should allow this to mean Any. The text of the PEP specifies a function;
To make type checkers aware of this, the file can contain the following code:
def __getattr__(name) -> Any: ...
Furthermore, based on the context of the PEP, I think this should only be allowed in stub files. If people want to Any attributes in Python files, it is less intrusive and cleaner imo to have them # type: ignore the file, but I think this needs discussion.
There was a problem hiding this comment.
Good point; I'll just make it ignore __getattr__ if it's not a function.
I'd prefer to keep allowing __getattr__ in .py files, not stubs, too. It's potentially useful (you might use it if you were to write annotated code for the user module from python/typeshed#1454) and it makes the implementation simpler if we don't have to check whether we're in a stub.
There was a problem hiding this comment.
I think it is an okay change as long as the PEP text is modified, as it does specify it to be used for stubs. I can see the benefit of using it in Python files too.
|
|
||
| [case testModuleLevelGetattribute] | ||
|
|
||
| def __getattribute__(): ... # E: __getattribute__ is not valid at the module level |
There was a problem hiding this comment.
Another potential test: an untyped __getattr__, and if the implementation is changed to refuse it in Python files, a test for that of course.
There was a problem hiding this comment.
Adding the first one.
- Ignore __getattr__ if it's not a function. - Test for untyped __getattr__ (and treat it as returning Any). Thanks for the comments!
|
I approved changes, but I think it'd be good to hear from @JukkaL or @gvanrossum on clarifying the PEP.
The implementation here adds support for it in not just stubs but also Python files. I think this is okay as long as the PEP clarifies this, or we think it is okay for MyPy to go beyond the PEP. Thoughts? |
|
In stubs you can can lie about various things. But I don't think we should allow |
|
Just pushed a change to only allow module-level |
|
It looks like this now conforms to what Guido said, so I am going to merge this. |
4 participants
Fixes #2496
PEP 484 specifies that stubs can add a global
__getattr__function to indicate that they have arbitrary other attributes. See python/typeshed#5 and python/typing#181.This implementation goes beyond the spec in one way:
__getattr__has a more precise return type thanAny, we use it.