Skip to content

Mypy forbids converting a method to a property in some cases #3913

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.

Sign up for GitHub

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

Jump to bottom
Closed
orenbenkiki opened this issue Sep 4, 2017 · 9 comments
Closed

Mypy forbids converting a method to a property in some cases #3913

orenbenkiki opened this issue Sep 4, 2017 · 9 comments

Comments

@orenbenkiki
Copy link

orenbenkiki commented Sep 4, 2017 •
edited
Loading

When type checking the following example.py:

from typing import Type
from typing import TypeVar


_Base = TypeVar('_Base', bound='Base')


class Meta(type):

    def method_to_access_a_special_instance(cls: Type[_Base]) -> _Base:
        # In the real code, this returns some special instance of cls instead of `None`.
        return None  # type: ignore

    @property
    def property_to_access_a_special_instance(cls: Type[_Base]) -> Base:
        # In the real code, this returns some special instance of cls instead of `None`.
        return None  # type: ignore


class Base(metaclass=Meta):
    pass


class Sub(Base):
    pass


a = Base()
a = Base.method_to_access_a_special_instance()  # Line 29

b = Sub()
b = Base.method_to_access_a_special_instance()  # Line 32
b = Sub.method_to_access_a_special_instance()   # Line 33

a = Base.property_to_access_a_special_instance  # Line 35
b = Sub.property_to_access_a_special_instance   # Line 36

Then mypy produces the following errors:

example.py:32: error: Incompatible types in assignment (expression has type "Base", variable has type "Sub")
example.py:35: error: Invalid method type
example.py:36: error: Incompatible types in assignment (expression has type "Base", variable has type "Sub")
example.py:36: error: Invalid method type

Expected results:

  • Lines 29 and 33 are accepted by mypy, as they should.

  • Line 32 is properly rejected because b has the type Sub and we assign a Base to it.

Unexpected results (bugs?):

  • Lines 35 and 36 are rejected with the message Invalid method type. This seems strange, as the only difference between them and the valid lines 29 and 33 is that the code is using a property instead of a method.

  • Line 36 also triggers an additional rejection claiming that the property returns a different type Base than the expected type Sub. Again this is strange since mypy correctly deduces the return type of the method, but gets confused and deduces a different type when using a property.

Workaround:

For now, I am using the method syntax, since the property syntax is rejected by mypy. However, this causes ugliness in the code. For example, I need to write something like MyClass.special_instance()[x], when I would much rather write MyClass.special_instance[x] instead.
@ilevkivskyi
Copy link
Member

It looks like a very similar error has been observed in #3625 (comment), so maybe this is a duplicate of #3223 modulo class -> metaclass? @elazarg are you working on this? It looks like your PR #3227 should help with this issue.

@elazarg
Copy link
Contributor

elazarg commented Sep 4, 2017

I can't verify this right now, but looks like it should be resolved by #3227.

@orenbenkiki
Copy link
Author

Good news. Thanks!

@elazarg
Copy link
Contributor

elazarg commented Sep 4, 2017 •
edited
Loading

Apparently this is not a precise duplicate, or #3227 is not a complete fix. Trying to use this with any version from 0.521/master/#3227, you get:

tmp.py:10: error: The erased type of self 'Type[tmp.Base]' is not a supertype of its class 'tmp.Meta'
tmp.py:15: error: The erased type of self 'Type[tmp.Base]' is not a supertype of its class 'tmp.Meta'
tmp.py:32: error: Incompatible types in assignment (expression has type "Base", variable has type "Sub")
tmp.py:36: error: Incompatible types in assignment (expression has type "Base", variable has type "Sub")

The first two errors are technically correct, although possibly too strict in practice. This is already discussed elsewhere.
The third is expected.
Line 35 is not rejected.
Line 36 looks like a bug, since it does not seem to perform proper instantiation (somehow it instantiate "itype" instead of "original_type").

I believe I should augment #3227 to fix this problem too.

@ilevkivskyi
Copy link
Member

I believe I should augment #3227 to fix this problem too.

This would be great!

@elazarg
Copy link
Contributor

elazarg commented Sep 24, 2017 •
edited
Loading

The problem seems to be that expand_type_by_instance takes an instance, and therefore "must" take itype as an argument, where we actually need it to take original_type which can be arbitrary type.

Specifically, adding print(functype) in analyze_var() reveals

def [_Base <: tmp.Base] (cls: Type[_Base`-1]) -> tmp.Base

which means the function is no longer generic at this point, its type variable are already instantiated by the statement t = expand_type_by_instance(typ, itype) earlier.

I assume the solution must be to use a function expand_type_by_type(), which will be a generalization of what's already happening inside bind_self() (in a somewhat ad-hoc manner).

(I write this here since I "reveal" it over and over again; also @ilevkivskyi I wonder if that makes sense to you)

So, this seems to require a different fix than #3227. I think its better not to augment it.

@ilevkivskyi
Copy link
Member

@elazarg

I assume the solution must be use a function expand_type_by_type(), which will be a generalization of what's already happening inside bind_self() (in a somewhat ad-hoc manner).

Or alternatively you could just generalize bind_self(), I don't remember the context, but I was thinking few times about such generalization.

It is OK (and maybe even desirable) if you do this in a separate PR.

@elazarg
Copy link
Contributor

elazarg commented Dec 3, 2017

Hmph. After another couple of hours debugging this, I've found the culprit:

class Meta(type):

    def method_to_access_a_special_instance(cls: Type[_Base]) -> _Base:
                                                                # ^Good.
        return None  # type: ignore

    @property
    def property_to_access_a_special_instance(cls: Type[_Base]) -> Base:
                                                                # ^oops, should be _Base.
        return None  # type: ignore

After correcting this typo, the output is:

tmp.py:10: error: The erased type of self 'Type[tmp.Base]' is not a supertype of its class 'tmp.Meta'
tmp.py:15: error: The erased type of self 'Type[tmp.Base]' is not a supertype of its class 'tmp.Meta'
tmp.py:32: error: Incompatible types in assignment (expression has type "Base", variable has type "Sub")

Which is as expected, up to the Meta vs. Type[T] strictness issue discussed in #3625. I guess it should be recommended not to use this naming scheme for type variables.

I believe this issue can be closed now.

@orenbenkiki
Copy link
Author

Oops indeed. My bad.

Yes, when I replace Base with _Base in the property definition, then mypy version 0.501 complains Invalid method type on each access of the property, since it tries to match Type[Base-1] with Meta and fails, so it seems like a duplicate of #3625.

I guess this should be closed as a duplicate, then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@orenbenkiki @gvanrossum @elazarg @ilevkivskyi