python / mypy

Optional static typing for Python
https://www.mypy-lang.org/
Other
18.22k stars 2.78k forks source link

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

Open sixolet opened 7 years ago

sixolet commented 7 years ago

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 commented 7 years ago

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

gvanrossum commented 7 years ago

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.

sixolet commented 7 years ago

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.

gvanrossum commented 7 years ago

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

srittau commented 6 years ago

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.

dacut commented 5 years ago

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).

ilevkivskyi commented 5 years ago

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 commented 5 years ago

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

chadrik commented 5 years ago

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__
ilevkivskyi commented 4 years ago

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.

joelberkeley-secondmind commented 4 years ago

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

warsaw commented 4 years ago

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.

dacut commented 4 years ago

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.

guillp commented 4 years ago

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]")

saroad2 commented 4 years ago

following

netokey commented 4 years ago

following

hauntsaninja commented 4 years ago

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

viblo commented 4 years ago

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)
gvanrossum commented 4 years ago

Use ‘Any’.

viblo commented 4 years ago

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.

gvanrossum commented 4 years ago

Try getting help on gitter

Dr-Irv commented 3 years ago

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

TiemenSch commented 3 years ago

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"
ztane commented 3 years ago

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'
KelSolaar commented 2 years ago

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

neg3ntropy commented 2 years ago

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.

ajnelson-nist commented 2 years ago

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.

gvanrossum commented 2 years ago

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

Dr-Irv commented 2 years ago

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

erictraut commented 2 years ago

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.

neg3ntropy commented 2 years ago

I beg forgiveness, but I will go a bit meta and off topic.

After putting a few days into actualizing an untyped python 3.6 project to typed python 3.10 I have to say the developer experience it's just still bad.

I don't have much to contribute, except my own experience on type systems: there's one page to be taken from the typescript book and that is: better unsafe than noisy

I would have killed for this to be great with python. Last time I evaluated it on 3.6 it just was not worth for web development. It's been a while and there's still a lot of hurdles that hinder adoption. I have great confidence that the next one will be the one that makes it workable, with the Self type and not requiring the future imports for forward refs.

My only suggestion for the tooling is to review priorities so that anything too strict bugging people takes precedence on anything too lax bugging programs. It worked for typescript and we have better readability and test coverage for sure.

And that syntax matters.

This particular issue is just one of few I will be watching before committing myself fully to work with typechecking on and I could even work around it easily, but it shocked me that it was this old.

Amazing that pyright supports it, but if this is a matter of disallowing perversion and not full blown incorrectness, why is this enabled in the basics checks?

gvanrossum commented 2 years ago

I beg forgiveness, but I will go a bit meta and off topic.

There are many places where this rant would have been appropriate (e.g. posted to typing-sig, python-dev, python-ideas, or a new issue in https://github.com/python/typing, but not here, and issue about a very specific topic. Please respect the boundaries of our community.

gvanrossum commented 2 years ago

@erictraut

A couple of things to consider here.

(Hm, we've gone over this before, and we seem to be going around in circles. I'll give it one last try -- if I don't convince you and nobody else steps in we may have to take this to the SC...)

First, Python seems to treat property inconsistently.

I think Python (in the sense of the runtime, or the language as described in the reference manuals) is completely consistent. Writing a property (as opposed to a "mere attribute") is a shorthand for calling the correspondong descriptor's __set__ method, reading one calls __get__. No inconsistency there.

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.

That is merely one of the most common behaviors found in descriptors (the most common is probably "properties are read-only").

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.

The language has picked a lane, and it is the semantics based on __get__ and __set__.

Second, type checkers apply type narrowing when a value is assigned to an expression.

Clearly this is the crux of your argument.

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.

Yes, and mypy started this trend. Unfortunately it is not consistent with the runtime behavior. Now, mypy often is stricter than runtime behavior, but we try not to flag useful contructs (assuming they are statically typable). I think we made an error of judgment here -- the handling of @property in mypy was (at least originally) pretty hackily based on the handling of plain attributes under the assumption that setters taking a different set of types than getters are uncommon and undesirable. I think we just made a mistake with that assumption. Such setters are certainly not common, but they exist, and can be useful. (For various reasons involving compatibility with either older versions of a library or with an established API, you can't always choose to write an explicit function call in these cases, as seems to be your recommendation.)

This is a valuable assumption that reduces false positives in typed code.

But does it? Do you have numbers on how often narrowing the type upon assignment to a property actually avoids a false positive?

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.

As I have argued there are other points of view. This issue wouldn't be so persistent if it was easy to do so.

Dr-Irv commented 2 years ago

@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.

If you use the example shown at https://github.com/python/mypy/issues/3004#issuecomment-980453107 then on the line that says def x(self, delta: Interval) -> None:, you get the message:

Property setter value type is not assignable to the getter return type
  Type "Interval" cannot be assigned to type "timedelta"
    "int" is incompatible with "timedelta"

I did the override and that made the message go away. IMHO, the default on this one with "basic" type checking for pyright should be "none" or "warning", not "error", mainly because the use case of having a setter of a different type than the getter is common enough (as illustrated by this long thread) and I agree with what @gvanrossum said here:

I think Python (in the sense of the runtime, or the language as described in the reference manuals) is completely consistent. Writing a property (as opposed to a "mere attribute") is a shorthand for calling the correspondong descriptor's set method, reading one calls get. No inconsistency there.

@erictraut wrote:

Personally, I think asymmetric properties are not a good idea because they break the mental model that most programmers have for properties.

"Most"? Do you have evidence on that? I think this depends on how you learned python, how long you have been programming in general, and how long you have been programming in python specifically.

IMHO, with pyright, the issue is what the default value of the setting reportPropertyTypeMismatch should be. Now that I know the override, I'm happy there. With mypy, there are really two issues:

  1. mypy should allow asymmetric getters/setters on properties. That seems to be something that requires some programming as indicated by @gvanrossum here: https://github.com/python/mypy/issues/3004#issuecomment-980455693
  2. Once mypy allows that capability, then there should likely be a setting to control whether it is reported or not. And then there needs to be a decision as to what the default value of that setting should be ("ignore", 'warning" or "error")
neg3ntropy commented 2 years ago

There are many places where this rant would have been appropriate

Sorry, a moment of frustration, but with good intentions

erictraut commented 2 years ago

@gvanrossum

Hm, we've gone over this before

I don't remember having a discussion with you previously about asymmetric properties. We did have a discussion about assignment-based type narrowing in the presence of asymmetric data descriptors in this thread. Perhaps that's what you're thinking of here? If I understand you correctly, you're saying that properties should be treated like any other data descriptors. That's reasonable since properties are implemented using the data descriptor mechanism.

I think Python (in the sense of the runtime, or the language as described in the reference manuals) is completely consistent.

I'm referring to other inconsistencies where mypy treats attributes and properties as equivalent, such as override checks and protocol matching checks. If we're in agreement that properties have semantics and behaviors that are distinct from attributes, can we also then agree that an attempt to override a property with an attribute or vice versa should produce an error? Likewise, can we agree that a protocol that includes a property is not compatible with a class that defines an attribute of the same name and vice versa?

I think we just made a mistake with that assumption.

Thanks for clarifying. It sounds like your viewpoint has evolved since this thread started. (Or maybe I misinterpreted what you meant when you said "...decided that it's a perverse style that we just won't support".)

Do you have numbers on how often narrowing the type upon assignment to a property actually avoids a false positive?

No, I haven't conducted an experiment to gather that data. The answer will depend on the approach we decide to take. If we decide that assignment-based type narrowing is allowed for symmetric properties but not for asymmetric properties, the impact will be much more limited. If we decide to never apply assignment-based type narrowing for any property, that would be a big change and would be much more disruptive. I would therefore recommend going with the former approach. If there is a push for the latter, I would recommend doing the experiment and gathering data before proceeding because I think we'll find that it breaks a lot of existing code.

@Dr-Irv,

"Most"? Do you have evidence on that?

This assertion is based on the interactions I've had with Python developers over past several years. This shouldn't be a surprise if you look at how property is commonly explained to new Python programmers. Search for Python @property on the web, and you'll find many tutorials, articles and videos that describe properties in a manner that establishes this mental model. For example, here's the first definition that google returns:

What is a property in Python? The property() method in Python provides an interface to instance attributes. It encapsulates instance attributes and provides a property, same as Java and C#.

the issue is what the default value of the setting reportPropertyTypeMismatch should be

If the consensus here is that asymmetric properties are an intended use case, then I agree that the reportPropertyTypeMismatch check should not be enabled by default.

Here's a summary of what I think this thread is concluding:

  1. Properties (and data descriptors in general) in Python allow for asymmetric getters and setters. This is valid and intended, and type checkers should not (by default) flag this use case as an error.
  2. The semantics of a property are not meant to match the semantics of an attribute. They are similar, but they differ in some important ways.
  3. The semantics of a property match that of a data descriptor (since the former is built upon the latter). Any type-related rules that apply to one should also apply to the other. This includes type narrowing behaviors.
  4. A type checker should skip type narrowing upon assignment should for asymmetric properties and data descriptors. (Note: Pyright currently doesn't skip this. Mypy does, but the heuristic it uses to determine whether to skip is problematic as it leads to incorrect types. See https://github.com/python/mypy/issues/10399 for details.)
gvanrossum commented 2 years ago

(GitHub is having some issues so I'm replying, briefly, via email.)

My viewpoint has indeed evolved. Asymmetric properties and data descriptors are a fact of life and we should support them, and indeed I think properties are just a special case of data descriptors (though the version I can find in my ancient copy of typeshed seems to cut some corners -- it makes heavy use of Any, apparently relying on custom support in type checkers).

I'm not sure how to detect whether a property is asymmetric -- one could have something like this:

class C: @property def foo(self) -> float: return self._foo @foo.setter def foo(self, val: float) -> None: self._foo = float(val)

x = C() x.foo = 42 print(x.foo, type(x.foo)) # 42.0, float

Here inferring statically that x.foo has type int would be wrong. And I'm not sure what we could recommend to signal the "asymmetry" more forcefully here -- in fact, in a sense this code enforces symmetry.

Maybe we could place the burden on the user to write e.g.

@property def foo(self) -> int|float: ...

if they want to allow narrowing on assignment??

If we're in agreement that properties have semantics and behaviors that are distinct from attributes, can we also then agree that an attempt to override a property with an attribute or vice versa should produce an error?

It seems problematic to override a property with an attribute (how would you even make this work at runtime?) but it's not unusual to override a plain attribute with a property in a subclass, and it can be useful.

Likewise, can we agree that a protocol that includes a property is not compatible with a class that defines an attribute of the same name and vice versa?

No, because a protocol using a property is how you'd spell a read-only attribute. Protocols don't care about extra operations, so a class with a plain attribute named foo should be compatible with a protocol defining a property named foo. Not the other way around though.

Dr-Irv commented 2 years ago

This assertion is based on the interactions I've had with Python developers over past several years. This shouldn't be a surprise if you look at how property is commonly explained to new Python programmers. Search for Python @property on the web, and you'll find many tutorials, articles and videos that describe properties in a manner that establishes this mental model. For example, here's the first definition that google returns:

What is a property in Python? The property() method in Python provides an interface to instance attributes. It encapsulates instance attributes and provides a property, same as Java and C#.

Well, if you're an old timer like me, who has coded in multiple languages including Java, you're used to having multiple setters that take different types. So it's natural to do the same in python, and then you want the typing tools to support that.

the issue is what the default value of the setting reportPropertyTypeMismatch should be

If the consensus here is that asymmetric properties are an intended use case, then I agree that the reportPropertyTypeMismatch check should not be enabled by default.

If you do that in pyright, I'd be one happy camper!

erictraut commented 2 years ago

OK, I've made the change in pyright to disable reportPropertyTypeMismatch by default. I presume that someone will make the corresponding change in mypy so it no longer flags this situation as an error? Would it be useful to close this issue and create a separate mypy feature request that's specific to that change?

The "skip type narrowing for asymmetric properties and data descriptors" change is much more involved. I've opened an issue to track this change.

mscuthbert commented 2 years ago

I don't know if anyone has mentioned/discussed the possible restriction that was placed on properties in TypeScript when they added variant get/set types in 4.3 (May 2021). They have a restriction that the type of the getter must be an allowed type of the setter. Thus foo.x = foo.x should pass type checking.

(Of course it might still be an error if x.setter requires 0 < x < 10 and sets foo._x = x + 100, but it wouldn't be a type error...)

https://www.typescriptlang.org/docs/handbook/release-notes/typescript-4-3.html

rogerioo commented 1 year ago

Guys, is there any resolution about this discussion? Will mypy have coverage about cases when getters and setters have different type annotations?

mscuthbert commented 1 year ago

I wrote:

properties in TypeScript ... have a restriction that the type of the getter must be an allowed type of the setter.

This restriction was removed in TypeScript 5.1 https://devblogs.microsoft.com/typescript/announcing-typescript-5-1/#unrelated-types-for-getters-and-setters

akaszynski commented 10 months ago

Would it be possible to add a command line argument for mypy or an inline option to permit different types for setters and getters? Many repositories are working around this issue with # type: ignore, which is sort of the worst of both worlds because you're adding type hints but then ignoring them. Reusing an existing example:

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:  # type: allow-different-setter-type
        requests.post(f"https://example.com/{self.object_id}/value",
                      data=str(new_value).encode("utf-8"))
gentlegiantJGC commented 10 months ago

I also need an NDArray getter and a ArrayLike setter. I tried making a typed replacement for the property class but I kept hitting the already defined on line mypy issue on the setter.

The best I can do is give the getter, setter and deleter different method names and return the input method rather than the property. This has the side effect that the setter can be used as both a setter method and via the property setter with no extra code.

This isn't a solution to this issue but it is a workaround.

from __future__ import annotations
from typing import TypeVar, Any, Generic, Callable, overload

T = TypeVar("T")
GetT = TypeVar("GetT")
SetT = TypeVar("SetT")

class TypedProperty(Generic[GetT, SetT]):
    """
    Type hinting with the vanilla property does not support having a different type on the setter.
    See https://github.com/python/mypy/issues/3004
    This is a custom property implementation that supports independently typing the getter and setter to appease mypy.

    Note that the getter, setter and deleter return the method they were given and not a property instance.
    This was to appease mypy complaining about overriding variables.
    This has a side effect that a setter can be used as a setter or the method

    >>> class MyClass:
    >>>     def __init__(self) -> None:
    >>>         self._value = 1
    >>>
    >>>     @TypedProperty[int, int | float]
    >>>     def value(self) -> int:
    >>>         return self._value
    >>>
    >>>     @value.setter
    >>>     def set_value(self, val: int | float) -> None:
    >>>         self._value = int(val)
    >>>
    >>>     @value.deleter
    >>>     def del_value(self) -> None:
    >>>         del self._value
    >>>
    >>>
    >>> inst = MyClass()
    >>> assert inst.value == 1
    >>> inst.value = 2
    >>> assert inst.value == 2
    >>> inst.set_value(3)
    >>> assert inst.value == 3
    >>> del inst.value
    >>> try:
    >>>     inst.value
    >>> except AttributeError:
    >>>     pass
    >>> else:
    >>>     raise Exception
    >>> inst.value = 4
    >>> assert inst.value == 4

    If you want the original methods to be private then just prefix them with an underscore.
    """

    fget: Callable[[Any], GetT] | None
    fset: Callable[[Any, SetT], None] | None
    fdel: Callable[[Any], None] | None

    def __init__(
        self,
        fget: Callable[[Any], GetT] | None = None,
        fset: Callable[[Any, SetT], None] | None = None,
        fdel: Callable[[Any], None] | None = None,
        doc: str | None = None,
    ) -> None:
        self.fget = fget
        self.fset = fset
        self.fdel = fdel
        if doc is None and fget is not None:
            doc = fget.__doc__
        self.__doc__ = doc
        self._name = ""

    def __set_name__(self, owner: Any, name: str) -> None:
        self._name = name

    @overload
    def __get__(self, obj: None, objtype: None) -> TypedProperty[GetT, SetT]: ...

    @overload
    def __get__(self, obj: object, objtype: type[object]) -> GetT: ...

    def __get__(self, obj: Any, objtype: Any = None) -> GetT | TypedProperty[GetT, SetT]:
        if obj is None:
            return self
        if self.fget is None:
            raise AttributeError(f"property '{self._name}' has no getter")
        return self.fget(obj)

    def __set__(self, obj: Any, value: SetT) -> None:
        if self.fset is None:
            raise AttributeError(f"property '{self._name}' has no setter")
        self.fset(obj, value)

    def __delete__(self, obj: Any) -> None:
        if self.fdel is None:
            raise AttributeError(f"property '{self._name}' has no deleter")
        self.fdel(obj)

    def getter(self, fget: Callable[[Any], GetT]) -> Callable[[Any], GetT]:
        self.fget = fget
        return fget

    def setter(self, fset: Callable[[Any, SetT], None]) -> Callable[[Any, SetT], None]:
        self.fset = fset
        return fset

    def deleter(self, fdel: Callable[[Any], None]) -> Callable[[Any], None]:
        self.fdel = fdel
        return fdel

This is based on the example from the descriptor doc page.

It can be used like this. If you don't want separate setter and deleter methods then just make them private with an underscore prefix.

class Test:
    def __init__(self) -> None:
        self._value = 1

    @TypedProperty[int, int | float]
    def value(self) -> int:
        return self._value

    @value.setter
    def set_value(self, val: int | float) -> None:
        self._value = int(val)

    @value.deleter
    def del_value(self) -> None:
        del self._value

t = Test()
assert t.value == 1
t.value = 2
assert t.value == 2
t.set_value(3)
assert t.value == 3
del t.value
try:
    t.value
except AttributeError:
    pass
else:
    raise Exception
t.value = 4
assert t.value == 4

Edit: Fixed some type hinting

ks-mw commented 9 months ago

Would it be possible to add a command line argument for mypy or an inline option to permit different types for setters and getters? Many repositories are working around this issue with # type: ignore, which is sort of the worst of both worlds because you're adding type hints but then ignoring them. Reusing an existing example:

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:  # type: allow-different-setter-type
        requests.post(f"https://example.com/{self.object_id}/value",
                      data=str(new_value).encode("utf-8"))

I second this request. Allowing different types would allows us to make our API more convenient for our end users. They do not care if it is perverse or not ;) The type checker should check for errors, not try to enforce a specific coding style onto us. Complaining about different types for assignment/reading of a property is more the task of the linter I would say.

analog-cbarber commented 9 months ago

I don't see this error when using 1.7, so perhaps this was fixed when #16411 was closed?

philipp-horstenkamp commented 9 months ago

I don't see this error when using 1.7, so perhaps this was fixed when #16411 was closed?

I think it was closed because its an duplicat / not planned.

Dreamsorcerer commented 9 months ago

The type checker should check for errors, not try to enforce a specific coding style onto us.

I think you misunderstand the comments. If it is a perverse style, not used in many projects, that doesn't mean mypy shouldn't be able to type check it, but it means that it's probably not worth spending (the very limited) development time on the feature when there are hundreds of higher priority things to be worked on. It's possibly also not worth doing at all, if it adds too much complexity to the codebase that needs to be maintained in future. (I'm not familiar with the codebase, but assuming the latter is not true, then you can always try implementing the feature yourself and getting it merged in).

ks-mw commented 9 months ago

I think you misunderstand the comments.

Yes, I did. Ok, that is an argument I understand.

analog-cbarber commented 9 months ago

Whatever the reason, I don't see mypy generating errors about this right now.

It is pretty clear that this is NOT a perverse style and should be supported to some degree.