Skip to content
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

Descriptor as class attribute is misunderstood #4472

Closed
gvanrossum opened this issue Jan 16, 2018 · 10 comments
Closed

Descriptor as class attribute is misunderstood #4472

gvanrossum opened this issue Jan 16, 2018 · 10 comments
Labels

Comments

@gvanrossum
Copy link
Member

A descriptor is something with a __get__ method; the return type of __get__ defines the type of the corresponding instance attribute. However the corresponding class attribute ought to be inspectable just as if it was an instance of the descriptor class; right now it is treated as the same type as the instance attribute. Example:

class Descr:
    def __get__(self, *args) -> int: pass
    name: str = 'booh'

class C:
    a = Descr()

reveal_type(C().a)  # int
reveal_type(C.a.name)  # Any # E: "int" has no attribute "name"
@gvanrossum
Copy link
Member Author

gvanrossum commented Jan 16, 2018

There's a way to make the type of C.a be different from the the type of C().a but it is complicated and always seems to come out as Any:

from typing import Any, Generic, overload, Type, TypeVar

T = TypeVar('T')

class Descr(Generic[T]):
    @overload
    def __get__(self, obj: T, cls: Type[T]) -> int: pass
    @overload
    ##def __get__(self, obj: None, cls: Type[T]) -> Type[T]: pass  # What I originally wrote
    def __get__(self, obj: None, cls: Type[T]) -> Descr[T]: pass  # What it should be
    def __get__(self, obj: Any, cls: Any) -> object: pass
    name: str = 'booh'

class C:
    a = Descr['C']()

reveal_type(C().a)  # int
reveal_type(C.a)  # Any
reveal_type(C.a.name)  # Any

I wish the last two revelations to be C and str, respectively. Making Descr non-generic and just putting C instead of T makes no difference.

@ilevkivskyi
Copy link
Member

I think this is rather a bug in descriptor implementation. Probably there is just a missing special case in checkmember.py for things like Type[...], Callable with .is_type_obj() returning True etc.

@ilevkivskyi ilevkivskyi added bug mypy got something wrong priority-1-normal labels Jan 16, 2018
@gvanrossum
Copy link
Member Author

Descriptors are defined to also be called for class attribute requests. That was perhaps a mistake in the design of descriptors but that was ages ago. So technically we need the overload (and typically also the generics, alas). I wouldn't mind if we allowed omitting the overload and somehow ignored the descriptor for class attributes though -- I can't think of a use case for a descriptor that returns something other than the descriptor itself when called for a class attribute. Or perhaps require an explicit overload with None as the type for the first argument to trigger its use for class overloads. (These ideas may be too pragmatic though.)

@ilevkivskyi
Copy link
Member

OK, I see. After re-reading the docs it looks like the overload with instance=None seems to be most "precise", but it is probably currently blocked by #3295 (overloads on Nona are problematic) and this is probably why you see Anys.

We can consider returning descriptor object itself automatically for class access as a temporary measure then.

@gvanrossum
Copy link
Member Author

gvanrossum commented Jan 17, 2018 via email

@Michael0x2a
Copy link
Collaborator

This is maybe partially resolved?

@gvanrossum's first example still doesn't work, but the second example's output is a little better now after the overload-related changes. Running mypy master now results in:

test.py:16: error: Revealed type is 'builtins.int'
test.py:17: error: Revealed type is 'Type[test.C*]'
test.py:18: error: Revealed type is 'Any'
test.py:18: error: "Type[C]" has no attribute "name"

That said, maybe I'm misunderstanding something about how descriptors are supposed to work, but shouldn't the second alternative have a return type of Descr[T], not Type[T]? Making that change results in the following output:

test.py:16: error: Revealed type is 'builtins.int'
test.py:17: error: Revealed type is 'test.Descr[test.C*]'
test.py:18: error: Revealed type is 'builtins.str'

@gvanrossum
Copy link
Member Author

You're right, the second example should have -> Descr[T] for the second overload. Too bad the first example doesn't work yet.

@ilevkivskyi
Copy link
Member

Too bad the first example doesn't work yet.

But how do you propose to fix this? Should we special case __get__ access on classes if its signature is not an overload?

@gvanrossum
Copy link
Member Author

Hm, I've got a feeling I might have been wrong. The signature for __get__ should be overloaded to match the runtime behavior, like the corrected 2nd example. (I've now corrected it in place to avoid confusion.)

@thejohnfreeman
Copy link

A use case for class property descriptor that is evaluated and does not return itself:

The Conan package manager uses recipes that are classes, with attributes that are class (not instance) attributes. I didn't choose this design, but I have to deal with it. Descriptors give me the ability to define lazy attributes that compute their value, but only when accessed (because the computation is expensive).

There is already an open issue for it: #2563

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants