mypy icon indicating copy to clipboard operation
mypy copied to clipboard

What to do about setters of a different type than their property?

Open sixolet opened this issue 8 years ago • 52 comments

Consider this code:

import typing
class A:
    @property
    def f(self) -> int:
        return 1
    @f.setter  # Possible error for defining a setter that doesn't accept the type the @property returns
    def f(self, x: str) -> None:
        pass
a = A()
a.f = ''  # Possibly not an error, but currently mypy gives an error
a.f = 1  # Certainly should be an error, unless we disallowed 
         # defining the setter this way in the first place.
reveal_type(a.f)  # E: Revealed type is 'builtins.int'

Whatever we decide, I'm happy to build a PR; I have this code loaded into my head.

sixolet avatar Mar 15 '17 01:03 sixolet

In other words "with arbitrary setters, you can have an lvalue be of a different type than its corresponding rvalue"

sixolet avatar Mar 15 '17 01:03 sixolet

IIRC we debated a similar issues when descriptors were added and decided that it's a perverse style that we just won't support. If you really have this you can add # type: ignore.

gvanrossum avatar Mar 15 '17 03:03 gvanrossum

Right now, you have to # type: ignore the line that says a.f = '', but not the definition. Should you have to # type: ignore the setter definition too? My inclination is yes, but I'm not sure of this.

sixolet avatar Mar 15 '17 18:03 sixolet

What does a discrepancy between __get__ and __set__ (when using as a descriptor) do? I think that's the example to follow.

gvanrossum avatar Mar 15 '17 18:03 gvanrossum

One real-life case where I encountered this a few times now is a normalizing property like this:

from typing import Set, Iterable


class Foo:

    def __init__(self) -> None:
        self._foo = set()  # type: Set[int]

    @property
    def foo(self) -> Set[int]:
        return self._foo

    @foo.setter
    def foo(self, v: Iterable[int]) -> None:
        self._foo = set(v)


Foo().foo = [1, 2, 3]

I like this implementation, because it allows a user of the cleverly named class Foo not to care about the exact type of the property, while Foo can use the best representation internally, while also giving additional guarantees when accessing the property.

Using the same type in the getter and setter would complicate the life of its users.

srittau avatar Feb 23 '18 13:02 srittau

I disagree that this is "perverse" -- contravariance in the setter is one example, e.g. where the getter returns Set[x] and the setter takes Collection[x]:

from typing import Collection, Set

class X():
    @property
    def hello(self) -> Set[str]:
        return {"x", "y"}
    
    @hello.setter
    def hello(self, value: Collection[str]) -> None:
        pass

x = X()
x.hello = ["1", "2", "3"]
% mypy --version
mypy 0.701
% mypy mypy3004.py
mypy3004.py:13: error: Incompatible types in assignment (expression has type "List[str]", variable has type "Set[str]")

In my current case, I'm being even stricter in the getter, returning FrozenSet[x] to prevent accidental misuse of the returned collection (thinking it will change the attribute in the object itself).

dacut avatar May 19 '19 01:05 dacut

What does a discrepancy between __get__ and __set__ (when using as a descriptor) do? I think that's the example to follow.

Descriptors actually support different types as one would expect. However, properties were implemented before descriptors (using some heavy special-casing) so they don't support this.

I think this is a valid feature to support. I have seen this a lot recently in internal code bases, mostly in context similar to example by @srittau (canonical representation). This however may be tricky to implement (because of the special-casing I mentioned). Maybe a better strategy would be to just make property a regular descriptor in mypy (I believe we already have an issue for that), then this will be supported automatically.

ilevkivskyi avatar May 19 '19 09:05 ilevkivskyi

FTR, the main issue about properties is https://github.com/python/mypy/issues/220

ilevkivskyi avatar May 19 '19 09:05 ilevkivskyi

Here's another example that I don't find terribly perverse. Using None to indicate "use the default value":

class A:

    def __init__(self, label=None):
        # type: (Optional[str]) -> None
        self._user_label = None  # type: Optional[str]
        self.label = label

    @property
    def label(self):
        # type: () -> str
        return self._user_label or self.default_label()

    @label.setter
    def label(self, value):
        # type: (Optional[str]) -> None
        self._user_label = value

    def default_label(self):
        # type: () -> str
        return self.__class__.__name__

chadrik avatar Sep 09 '19 19:09 chadrik

I am going to remove the "needs discussion" label. IMO it is now pretty clear we should support this, the only discussion is what is the best way to implement this.

ilevkivskyi avatar Dec 05 '19 10:12 ilevkivskyi

perhaps I'm missing sth, but doesn't that mean you can get

class X: pass

class Foo:
   @property
   def foo(self) -> int:
       ...

   @foo.setter
   def foo(self, o: Union[X, int]) -> None:
       ...

foo = Foo()
x = X()
foo.bar = x
assert foo.bar != x

I wonder if there are cases where this makes sense (e.g. where equality still holds - perhaps this can work in the Iterable/Set example), but others where I also feel it seems odd e.g. using None for default values

My request would be for there to be a flag to turn this on, and it to be off by default

joelberkeley-secondmind avatar May 14 '20 12:05 joelberkeley-secondmind

I think this is the problem I am encountering now with mypy 0.781. Here's my distilled example:

from datetime import timedelta
from typing import Union

Interval = Union[timedelta, int]


class Foo:
    def __init__(self):
        self._x = timedelta(seconds=15)

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

    @x.setter
    def x(self, delta: Interval) -> None:
        if isinstance(delta, timedelta):
            self.x = delta
        else:
            self.x = timedelta(seconds=delta)


foo = Foo()
foo.x = 7
foo.x = timedelta(seconds=8)

And the error I'm getting:

% mypy foo.py 
foo.py:24: error: Incompatible types in assignment (expression has type "int", variable has type "timedelta")
Found 1 error in 1 file (checked 1 source file)

The idea behind the x property is that its type "is" a timedelta but you can set it with an int or a timedelta and the former always gets coerced to the latter. I think those are pretty pedestrian semantics!

Seems like this is the same problem described in this issue. I can understand that mypy may not be able to infer this.

warsaw avatar Jun 19 '20 19:06 warsaw

I wonder if there are cases where this makes sense (e.g. where equality still holds - perhaps this can work in the Iterable/Set example), but others where I also feel it seems odd e.g. using None for default values

My request would be for there to be a flag to turn this on, and it to be off by default

@joelberkeley-pio, what you're asking for is a Python feature, not a MyPy feature. This is part of the whole point of properties -- to divorce the getters and setters from exposing bare values of members, so we don't need to have setX() and getX() like in Java.

For example, this is also valid Python (and, apart from the contrived simplification, actually used in some libraries):

from typing import Union
import requests

class RemoteObject:
    def __init__(self, object_id: str):
        self.object_id = object_id

    @property
    def value(self) -> float:
        return float(requests.get(f"https://example.com/{self.object_id}/value").text)

    @value.setter
    def value(self, new_value: Union[int, float]) -> None:
        requests.post(f"https://example.com/{self.object_id}/value",
                      data=str(new_value).encode("utf-8"))

Anything could happen on the remote server between the time the value is set and retrieved. There is absolutely no expectation that the values should be identical.

dacut avatar Jun 19 '20 22:06 dacut

Please allow me to add another example of that same issue, for a use case which I believe is sane:

from typing import Any, Optional, Union

class Foo:
    def __init__(self, foo: str, **kwargs: Any) -> None:
        self.foo = foo

class Bar:
    def __init__(self, foo: Union[str, Foo] = None) -> None:
        self.foo = foo

    @property
    def foo(self) -> Optional[Foo]:
        return self._foo

    @foo.setter
    def foo(self, value: Union[str, Foo]) -> None:
        if value is not None and not isinstance(value, Foo):
            value = Foo(value)
        self._foo = value

This gives: mypy_property.py:9: error: Incompatible types in assignment (expression has type "Union[str, Foo, None]", variable has type "Optional[Foo]")

guillp avatar Jul 02 '20 17:07 guillp

following

saroad2 avatar Aug 25 '20 07:08 saroad2

following

netokey avatar Aug 28 '20 02:08 netokey

Github has a subscribe button for this purpose that creates less noise.

hauntsaninja avatar Aug 28 '20 05:08 hauntsaninja

Is there some workaround for this problem which does not require the consumer of the property to use # type: ignore on each property usage? I have a library Im trying to add type hints to, similar to the (very simplified) code below, and this problem comes up a lot.

class Vec:
    x: float = 0
    y: float = 0

    def __init__(self, x, y):
        self.x = x
        self.y = y

class A:
    @property
    def position(self):
        return self._position

    @position.setter
    def position(self, v):
        if isinstance(v, Vec):
            self._position = v
        else:
            self._position = Vec(v[0], v[1])

a = A()
a.position = (1, 2)
print(a.position.x)

viblo avatar Sep 02 '20 14:09 viblo

Use ‘Any’.

gvanrossum avatar Sep 02 '20 14:09 gvanrossum

Thanks for the quick reply! Unfortunately I still cant figure it out.

I modified my example like below:

from typing import Any

class Vec:
    x: float = 0
    y: float = 0

    def __init__(self, x: float, y: float) -> None:
        self.x = x
        self.y = y

class A:
    _position: Vec

    @property
    def position(self) -> Any:
        return self._position

    @position.setter
    def position(self, v: Any) -> None:
        if isinstance(v, Vec):
            self._position = v
        else:
            self._position = Vec(v[0], v[1])

a = A()
a.position = (1, 2)
reveal_type(a.position)
print(a.position.x)

Now it works in Pylance/pyright, but mypy still complains. In pyright the revealed type is Any, but in mypy it says builtins.list[Any] and I still get the type error on the last line.

viblo avatar Sep 02 '20 14:09 viblo

Try getting help on gitter

gvanrossum avatar Sep 02 '20 15:09 gvanrossum

There is a way to work around the example given in the comment above by @srittau https://github.com/python/mypy/issues/3004#issuecomment-368007795

from typing import Set, Iterable

class Foo:
    def __init__(self) -> None:
        self._foo = set()  # type: Set[int]

    def get_foo(self) -> Set[int]:
        return self._foo

    def set_foo(self, v: Iterable[int]) -> None:
        self._foo = set(v)

    foo = property(get_foo, set_foo)


a = Foo()
a.foo = [1, 2, 3]
print(a.foo)

Instead of using the property decorators, just use the property class

Dr-Irv avatar Sep 20 '20 12:09 Dr-Irv

There is a way to work around the example given in the comment above by @srittau #3004 (comment)

from typing import Set, Iterable

class Foo:
    def __init__(self) -> None:
        self._foo = set()  # type: Set[int]

    def get_foo(self) -> Set[int]:
        return self._foo

    def set_foo(self, v: Iterable[int]) -> None:
        self._foo = set(v)

    foo = property(get_foo, set_foo)


a = Foo()
a.foo = [1, 2, 3]
print(a.foo)

Instead of using the property decorators, just use the property class

That removes the mypy error, but I believe you also won't get an error when you type

a.foo = "type should fail"

TiemenSch avatar Nov 12 '20 11:11 TiemenSch

This pattern is used by WebOb in many places, for example:

>>> from webob import Response
>>> r = Response()
>>> r.status
'200 OK'
>>> r.status = 404
>>> r.status
'404 Not Found'

ztane avatar Dec 03 '20 16:12 ztane

Hello,

I wanted to add another use case common with Numpy. Often, a definition might accept an ArrayLike type, i.e. any object that can be coerced to a ndarray.

For example:

def add_array(a: ArrayLike, b: ArrayLike) -> NDarray:
    return np.asarray(a) + np.asarray(b)

By extension, one would like to be able to do the following:

class LinearInterpolator:
    def __init__(self, x: ArrayLike, y: ArrayLike):
        self._x: NDArray = np.array([-np.inf, np.inf])
        self._y: NDArray = np.array([-np.inf, np.inf])

        self.x = x
        self.y = y

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

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

    @property
    def y(self) -> NDArray:
        return self._y

    @y.setter
    def y(self, value: ArrayLike):
        self._y = np.asarray(value)

Cheers,

Thomas

KelSolaar avatar Nov 19 '21 22:11 KelSolaar

I remember a lot of discussion around Properties vs Getter and Setters and how python was supposed to do it better with properties: start easy, then add property methods if necessary, figure it out when you get the problem. It was the python way.

No python developer prefers to do a x.set_val(v) rather than a x.val = v, so properties are used a lot and no one cared if what you set is not what you get. That was perfecly fine with getters/setters and we were doing something even better, so what that the equal sign becomes a little weird.

But now, it's suddenly bad. Cannot be typed.

So yeah maybe we say it's "perverse" to do that with a property. Fine.

Let's limit those to simple things that really assign what you set. Ok.

Maybe don't even start to expose something as a property just in case that in the future you will want to do perverse stuff. You never know.

Just start putting boilerplate getters and setters right from the beginning. To be safe.

OMG the next step is java. Irony aside, either we optimized for the wrong thing and cultivated the wrong culture around properties, or we need this to support the perverse python programming that we taught for so long.

Or we say that typed python is just for weekend fun, by leaving issues open for 5 years.

Sorry again for the irony, I hope my point is understandable.

neg3ntropy avatar Nov 26 '21 15:11 neg3ntropy

It seems effort around resolving or implementing this thread has stalled. It might be beneficial to review base goals.

  1. Type signatures on the getter make guarantees of what a user will receive on a read.
  2. Type signatures on the setter guarantee that what a user writes is somehow expected by the implementation.

It has been a mostly reasonable behavior to date to have the signatures in set() and signatures out get() match exactly, but this thread has identified several use cases, that were acknowledged as worth supporting, where types unrelated to the getter could be provided to a setter.

Maybe the relationship between the getter signature and the setter signature should be adjusted. Is it a bad idea, and/or nightmarishly complex, to have a getter's types be decoupled from the setter's types?

For illustration, I'll repeat @warsaw 's example from 2020-06-19 (though I fixed what looked like a couple accidental infinite-recursion typos from the distilling, self.x = delta fixed to be self._x = delta):

from datetime import timedelta
from typing import Union

Interval = Union[timedelta, int]


class Foo:
    def __init__(self):
        self._x = timedelta(seconds=15)

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

    @x.setter
    def x(self, delta: Interval) -> None:
        if isinstance(delta, timedelta):
            self._x = delta
        else:
            self._x = timedelta(seconds=delta)


foo = Foo()
foo.x = 7
foo.x = timedelta(seconds=8)

These two lines:

    def x(self) -> timedelta:
    ...
    def x(self, delta: Interval) -> None:

should not have to have anything to do with each other. The latter would provide type information for the function body of the x setter. The former would provide an inferable type constraint on self._x. It's self._x that has the key type constraint, not explicitly written in __init__, but found from the getter. So long as the setter mutates self._x within the upper bound of timedelta, this example should be fine.

I appreciate this might be an overly simplistic solution. I, and probably the others who have watched this ticket the last almost five years, would like to understand what would stop the implementers from going ahead now and removing the getter/setter match logic. Maybe with a counterexample?

As an aside, my own interest in this comes from testing an old type-normalizing class, also related to time. mypy's not a fan of our getter returning only a MyTimeThing, but our setter helpfully also accepting a UNIX timestamp or a string. That is, the getter has signature def x(self) -> MyTimeThing, the setter has signature def x(self, value: Union[MyTimeThing, int, str]) -> None. I can see with my eyes that the setter's short body guarantees foo.x will return a MyTimeThing. But, mypy just sees the signature and then gives up.

ajnelson-nist avatar Nov 26 '21 22:11 ajnelson-nist

Yes, it’s a simple matter of programming. The desired behavior is clear.

gvanrossum avatar Nov 26 '21 22:11 gvanrossum

Maybe if mypy fixes this, we can get pyright to do the same......

Dr-Irv avatar Nov 26 '21 22:11 Dr-Irv

A couple of things to consider here.

First, Python seems to treat property inconsistently. In some circumstances, it is treated as semantically equivalent to an attribute. The mental model is that when a value is written to a property, that same value (or at least the same type) will be read from the property. In other circumstances, properties are treated as having semantics that differ significantly from attributes. Type asymmetry is a good example of this inconsistency. It would be good if we could "pick a lane" and stick to it. The inconsistency leads to confusion and unproductive debates.

Second, type checkers apply type narrowing when a value is assigned to an expression. This includes assignments to member access expressions like self.x = 3. Type checkers typically assume that the narrowed type is based on the last assignment. This is a valuable assumption that reduces false positives in typed code. This assumption is violated for asymmetric properties (and more generally, for asymmetric descriptors). Please take into account type narrowing behaviors (and the value they provide) as you consider the resolution of this issue. @ajnelson-nist, I mention this because your proposal above doesn't appear to consider this.

@Dr-Irv, I think pyright correctly handles asymmetric properties already, so I'm interested in what you meant by your comment above. In the sample that was provided at the start of this issue, pyright does not emit an error for the line a.f = "" and does emit an error for a.f = 1. Pyright does emit an error for an asymmetric property setter if reportPropertyTypeMismatch is enabled, but this check can be disabled if you want to use asymmetric properties in your code base.

Personally, I think asymmetric properties are not a good idea because they break the mental model that most programmers have for properties. I wouldn't go so far as to call the pattern "perverse", but it's highly undesirable IMO. If I want to expose an asymmetric getter/setter in my own code, I don't use a property. Instead, I expose a dedicated setter method to make it clear that its behavior is not consistent with the normal property behavior.

erictraut avatar Nov 26 '21 23:11 erictraut