Repository navigation
Overloads for property getter (instance vs. through owner) #5093
Description
The docs for object.__get__ specify that:
object.__get__(self, instance, owner)... owner is always the owner class, while instance is the instance that the attribute was accessed through, or None when the attribute is accessed through the owner.
The typeshed for pynamodb has:
class NumberAttribute(Attribute[Number]):
def __get__(self, instance: Any, owner: Any) -> Number: ...This allowed us to use PynamoDB idiomatically so that MyModel().my_number is considered a Union[int, float] rather than an instance of NumberAttribute. The underlying implementation might not use properties, but superficially it works ;)
Recent versions of PynamoDB added a conditional query syntax like this:
for my_model in MyModel.query(range_key_condition=MyModel.my_number > 42):
...I've tried representing it like this:
class NumberAttribute(Attribute[Number]):
@overload
def __get__(self, instance: object, owner: Any) -> Number: ...
@overload
def __get__(self, instance: None, owner: Any) -> NumberAttribute: ...but mypy seems to ignore the latter override.
Test code:
from pynamodb.attributes import NumberAttribute, ListAttribute
from pynamodb.models import Model
class MyModel(Model):
my_number = NumberAttribute(hash_key=True)
reveal_type(MyModel.my_number)
m1 = MyModel()
reveal_type(m1.my_number)Result:
pynamodb.py:9: error: Revealed type is 'Union[builtins.int, builtins.float]'
pynamodb.py:12: error: Revealed type is 'Union[builtins.int, builtins.float]'
Activity
JelleZijlstra commentedon May 22, 2018
Does it work if you reverse the order of the overloads?
ikonst commentedon May 22, 2018
results in:
Michael0x2a commentedon May 23, 2018
This should hopefully be fixed sometime in the near future: we're currently in the process of overhauling how mypy handles overloads in general, and the plan is to fix this along the way.
(Well, it's not so much "fix" as it is "special-case overloading descriptors to allow this", but whatever.)
There's also some related discussion here: #3757.
Michael0x2a commentedon Jun 8, 2018
I'm pretty certain this ought to be fixed on master now, though I'm not very familiar with the pynamodb stubs so can't say for certain.
@ikonst -- can you confirm?
ikonst commentedon Jun 9, 2018
This definition works:
This one also works:
(note the
Nonevariant being first)ilevkivskyi commentedon Jun 9, 2018
OK, so as I understand this one can be closed now.