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

General support for properties #220

Closed
JukkaL opened this issue Jul 13, 2013 · 30 comments
Closed

General support for properties #220

JukkaL opened this issue Jul 13, 2013 · 30 comments
Labels
bug mypy got something wrong false-positive mypy gave an error on correct code priority-1-normal

Comments

@JukkaL
Copy link
Collaborator

JukkaL commented Jul 13, 2013

Add support for type checking properties.

We should initially support at least the decorator syntax for read-only properties and the assignment syntax for both read-only and read-write properties, as these seem to be the most frequently used variants:

class A:
    @property
    def foo(self) -> int: return 2

    def get_bar(self) -> int: return 3
    def set_bar(self, value: int) -> None: ...
    bar = property(get_bar, set_bar)

Also support defining a deleter and a docstring, at least when using the call syntax.

@JukkaL
Copy link
Collaborator Author

JukkaL commented Jul 14, 2013

Generic descriptor support would also let us support properties, but the distinction between read-only and settable properties could be lost. A property would have type property[ObjT, ValT], where ObjT is the class that defines the property and ValT is the type of the property value.

In order to properly model settable properties, we should have something like property[ObjT, GetT, SetT], and allow SetT to be undefined. However, there is currently no way of letting the value of a type argument be undefined. We could also generally support default values for type variables, and undefined would a valid default value.

@JukkaL
Copy link
Collaborator Author

JukkaL commented Sep 21, 2013

Read-only properties using @property work, but not the other forms.

@ghost ghost assigned JukkaL Dec 24, 2013
@JukkaL JukkaL removed their assignment Jul 25, 2014
@JukkaL JukkaL added the priority label Oct 8, 2014
@thomascellerier
Copy link

Any progress on this? I am using mypy to check some new production (yes i know :)) python daemon and it worked fine until I started using properties. Do you know of any workaround? Even using the old properties notation i get issues with setters.
A property cannot be overloaded
'overload' decorator expected

Thanks for this great tool.

@gvanrossum
Copy link
Member

I'm guessing you're talking about an example like this:

class C:
    def __init__(self):
        self._x = None

    @property
    def x(self):
        """I'm the 'x' property."""
        return self._x

    @x.setter
    def x(self, value):
        self._x = value

    @x.deleter
    def x(self):
        del self._x

Indeed mypy goes crazy over this:

a.py, line 5: A property cannot be overloaded
a.py, line 5: 'overload' decorator expected
a.py, line 10: Name 'x' is not defined
a.py, line 10: 'overload' decorator expected
a.py, line 14: Name 'x' is not defined
a.py, line 14: 'overload' decorator expected

@JukkaL
Copy link
Collaborator Author

JukkaL commented Apr 5, 2015

Yeah, this hasn't been implemented yet. It's been on my to-do list for a long time, but since the code I've worked with haven't used setters much I've never gotten around to actually implementing this. The implementation should not be very difficult, though.

I'll try to get this working reasonably soon (though probably only after PyCon, i.e. after April 15th).

@JukkaL
Copy link
Collaborator Author

JukkaL commented Apr 5, 2015

I just pushed experimental setter support. The following code now works as expected:

class C:
    def __init__(self) -> None:
        self._x = 0

    @property
    def x(self) -> int:
        return self._x

    @x.setter
    def x(self, value: int) -> None:
        self._x = value

    @x.deleter
    def x(self) -> None:
        del self._x

c = C()
c.x = 2 * c.x
c.x = ''   # Error!

The implementation still needs refinement. For example, x = property(...) is not supported.

Let me know if/when you encounter additional issues.

@thomascellerier
Copy link

Thanks, the file containing the properties now typechecks.
However I have another file in the same module assigning the property to a variable which fails to typecheck with this error (given a property called x with a getter and setter):
a.py

class C:
    @property
    def x(self) -> Dict[str, Any]:
        return self._x

    @x.setter
    def x(self, value: Dict[str, Any]) -> None:
        self._x = value

b.py

def foo(c: a.C):
    x = c.x

Cannot determine type of 'x'.

@JukkaL
Copy link
Collaborator Author

JukkaL commented Apr 10, 2015

Do you have cyclic imports? Mypy has trouble with them (#481) in general. However, it should be pretty easy to fix mypy to support your example -- assuming that this actually is due to cyclic imports.

@NYKevin NYKevin mentioned this issue Jan 20, 2016
@JukkaL
Copy link
Collaborator Author

JukkaL commented Feb 3, 2016

This still doesn't work (reported by Tim Abbott):

class C:
    def __init__(self) -> None:
        self._x = 0

    @property
    def x(self) -> int:
        return self._x

    @x.setter
    def set_x(self, value: int) -> None:   # Note the name of the method! "def x" works.
        self._x = value

@JukkaL
Copy link
Collaborator Author

JukkaL commented Feb 3, 2016

The last example seems to be only valid in Python 2, not Python 3.

@Deimos
Copy link
Contributor

Deimos commented Mar 10, 2017

Here's some more examples of strange property-related behavior:

This code produces a mypy error as it should (the passwordconstructor argument was "accidentally" annotated as an int, but the property setter expects a str value):

class User:
    @property
    def password(self) -> str:
        raise AttributeError('Password is write-only')

    @property.setter
    def password(self, value: str) -> None:
        # TODO: add hashing
        self.password_hash = value

    def __init__(self, username: str, password: int) -> None:
        self.username = username
        self.password = password

Running mypy gives mypy_test.py:13: error: Incompatible types in assignment (expression has type "int", variable has type "str") as expected.

However, if I just rearrange the order of the methods so that the constructor is first and the property methods come after it, there's no error produced any more:

class User:
    def __init__(self, username: str, password: int) -> None:
        self.username = username
        self.password = password

    @property
    def password(self) -> str:
        raise AttributeError('Password is write-only')

    @property.setter
    def password(self, value: str) -> None:
        # TODO: add hashing
        self.password_hash = value

Running mypy on that code has no output.

In addition, even the "more correct" behavior from the original code was only because the property was annotated incorrectly. Because attempting to read .password always raises an error, the correct way to write this code should have been (annotation on the password constructor argument "fixed" to str as well):

from mypy_extensions import NoReturn

class User:
    @property
    def password(self) -> NoReturn:
        raise AttributeError('Password is write-only')

    @property.setter
    def password(self, value: str) -> None:
        # TODO: add hashing
        self.password_hash = value

    def __init__(self, username: str, password: str) -> None:
        self.username = username
        self.password = password

However, even though this code should now be correct, mypy produces mypy_test.py:15: error: Incompatible types in assignment (expression has type "str", variable has type NoReturn).

So it seems like mypy is using the return value of the "getter" to determine the value the "setter" can accept, but that won't necessarily always match up.

@gvanrossum
Copy link
Member

Hm, I think we ran into a similar issue (about potential discrepancies between getter and setter) for descriptors (#2266).

@funkyfuture
Copy link

funkyfuture commented Mar 15, 2017

edit: previously contained #3011

@gvanrossum
Copy link
Member

it'd be nice to support abc.abstractproperty as well.

Should already be supported, see #263.

@gvanrossum
Copy link
Member

@funkyfuture Please file a separate issue.

@gvanrossum gvanrossum removed this from the 0.5 milestone Mar 29, 2017
@douglas-treadwell
Copy link

douglas-treadwell commented Jul 11, 2017

@JukkaL

So I've seen this commit a103f25 that closed #263. It seems to check for exactly "property" or "abc.abstractproperty" for property support.

Is there a way to support non-stdlib property decorators such as SqlAlchemy's hybrid_property, which has getter, setter, deleter, as well as expression and comparator?

I'd rather not need to litter my code with # type: ignore.

@douglas-treadwell
Copy link

Also, I'd be happy to fix this myself if someone @JukkaL can give me enough context to do so in less time than it'd take just to fix it themselves.

@ilevkivskyi
Copy link
Member

Another thing that doesn't work came in #6045: accessing attributes of a property object in the class body (for example fset, fget etc). This can be probably partially solved by some special casing in checkmember.py.

@Deimos
Copy link
Contributor

Deimos commented Jun 22, 2019

In my comment above from March 9, 2017, I noted that putting __init__ before property getter/setters caused mypy to miss a (false-positive) error that it otherwise reported.

I don't know if it's useful information at all, but I just wanted to point out that the new semantic analyzer no longer has that particular issue, and it will report the same error (which isn't actually one) regardless of whether __init__ is at the top or bottom of the class.

Specifically, here's the code:

from mypy_extensions import NoReturn

class User:
    def __init__(self, username: str, password: str) -> None:
        self.username = username
        self.password = password

    @property
    def password(self) -> NoReturn:
        raise AttributeError('Password is write-only')

    @property.setter
    def password(self, value: str) -> None:
        # TODO: add hashing
        self.password_hash = value

And the output from both analyzers:

$ mypy test_mypy.py
$ mypy --new-semantic-analyzer test_mypy.py
test_mypy.py:6: error: Incompatible types in assignment (expression has type "str", variable has type "NoReturn")

@vitodsk

This comment has been minimized.

@gonzaloamadio
Copy link

I have this file


import operator
from functools import total_ordering, wraps

def _time_required(f):
    @wraps(f)
    def wrapper(self, other):
        if not isinstance(other, Time):
            raise TypeError("Can only operate on Time objects.")
        return f(self, other)
    return wrapper
def _int_required(f):
    @wraps(f)
    def wrapper(self, value):
        if not isinstance(value, int):
            raise TypeError("An integer is required.")
        return f(self, value)
    return wrapper
def _balance(a, b):
    if b >= 0:
        while b >= 60:
            a += 1
            b -= 60
    elif b < 0:
        while b < 0:
            a -= 1
            b += 60
    return a, b

@total_ordering
class Time:

    def __init__(self, h: int = 0, m: int = 0, s: int = 0):
        self._h = h
        self._m = m
        self._s = s

    @property
    def h(self):
        return self._h

    @h.setter
    @_int_required
    def h(self, value):
        self._h = value

    @property
    def m(self):
        return self._m

    @m.setter
    @_int_required
    def m(self, value):
        self._m = value
        self._h, self._m = _balance(self._h, self._m)

    @property
    def s(self):
        return self._s

    @s.setter
    @_int_required
    def s(self, value):
        self._s = value
        self._m, self._s = _balance(self._m, self._s)

    def _operation(self, other, method):
        h = method(self.h, other.h)
        m = method(self.m, other.m)
        s = method(self.s, other.s)
        return Time(h, m, s)

    @_time_required
    def __add__(self, other):
        return self._operation(other, operator.add)

    @_time_required
    def __sub__(self, other):
        return self._operation(other, operator.sub)

    @_time_required
    def __eq__(self, other):
        return (self.h == other.h and
                self.m == other.m and
                self.s == other.s)

    @_time_required
    def __lt__(self, other):
        if self.h < other.h:
            return True
        if self.h > other.h:
            return False
        if self.m < other.m:
            return True
        if self.m > other.m:
            return False
        return self.s < other.s

    def __repr__(self):
        return f"<Time {self.h:02}:{self.m:02}:{self.s:02}>"

if __name__ == '__main__':
    a = Time(14, 23, 10)
    try:
        a.h = "Hello, world!"
    except Exception as err:
        print(err)

And when I run mypy, I have the following errors.

class.py:47: error: Decorated property not supported     **<-- Line @h.setter**
class.py:56: error: Decorated property not supported     **<-- Line @m.settet**
class.py:66: error: Decorated property not supported     **<-- Line @s.setter**
class.py:110: error: Property "h" defined in "Time" is read-only  **<-- Line:  a.h = "Hello, world!"**

If I comment out the decorators under the setters, mypy check runs OK. No errors

@gvanrossum
Copy link
Member

@gonzaloamadio Thanks for the report. I can't help you with a quick fix. The problem is that property (both get and set functionality) is special-cased in mypy, and currently doesn't support additional decorators at all. Sorry!

@ZahlGraf
Copy link

ZahlGraf commented Dec 5, 2019

@gvanrossum I have the same issue. In my project I use additional decorators for properties very often. Thus mypy sends me hundreds of error with "error: Decorated property not supported".

First of all, I don't understand the error message. Does it mean mypy does not support decorated properties, or is decorating a property related to any code quality issue in general? Because if just mypy is not supporting decorated properties, but the pattern in general is not considered as bad practice, I don't know, why mypy is giving me an error here. It should be at maximum a warning right?

Second, if it is just a mypy issue, I really want to ignore it. Is there any way to ignore exactly this error on a global level? At the moment, I'm only aware of ignoring type errors for a single line, but since this pattern is extremely common in my code, I don't want to add the "ignore line" comment on hundreds of lines in my code.

Thanks in advance for any suggestions and assistance.

@JukkaL
Copy link
Collaborator Author

JukkaL commented Dec 5, 2019

Decorated property has a separate issue: #1362

@JukkaL
Copy link
Collaborator Author

JukkaL commented Dec 5, 2019

I'm closing as this covers several related issues, some of which have already been fixed, and it's hard to keep track of what's still open. I created separate issues for the remaining problems. Feel free to create additional issues if I missed something.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug mypy got something wrong false-positive mypy gave an error on correct code priority-1-normal
Projects
None yet
Development

No branches or pull requests