Support properties with setter type different from getter type #18510

ilevkivskyi · 2025-01-22T23:27:20Z

Note: this PR should be reviewed with "hide whitespace" option (in couple long functions I replace huge if x: ... with if not x: return; ... to reduce indent level).

The core logic is quite straightforward (almost trivial). The only couple things are:

We should be careful with binder (we simpy can't use it for properties with setter type different from getter type, since we don't know underlying property implementation)
We need to handle gracefully existing settable properties that are generated by plugins

The tricky part is subclassing and protocols. The summary is as following:

For protocols I simply implement everything the "correct way", i.e. for settable attributes (whether it is a variable or a settable property) compare getter types covariantly and setter types contravariantly. The tricky part here is generating meaningful error messages that are also not too verbose.
For subclassing I cannot simply do the same, because there is a flag about covariant mutable override, that is off by default. So instead what I do is if either subclass node, or superclass node is a "custom property" (i.e. a property with setter type different from getter type), then I use the "correct way", otherwise the old logic (i.e. flag dependent check) is used.

Two things that are not implemented are multiple inheritance, and new generic syntax (inferred variance). In these cases setter types are simply ignored. There is nothing conceptually difficult about these, I simply run out of steam (and the PR is getting big). I left TODOs in code for these. In most cases these will generate false negatives (and they are already kind of corner cases) so I think it is OK to postpone these indefinitely.

github-actions · 2025-01-22T23:45:41Z

Diff from mypy_primer, showing the effect of this PR on open source code:

optuna (https://github.com/optuna/optuna)
+ optuna/trial/_trial.py:692: error: Unused "type: ignore" comment  [unused-ignore]

python-htmlgen (https://github.com/srittau/python-htmlgen)
+ test_htmlgen/element.py:218: error: Unused "type: ignore" comment  [unused-ignore]
+ test_htmlgen/element.py:224: error: Unused "type: ignore" comment  [unused-ignore]
+ test_htmlgen/element.py:225: error: Unused "type: ignore" comment  [unused-ignore]
+ test_htmlgen/element.py:254: error: Unused "type: ignore" comment  [unused-ignore]

operator (https://github.com/canonical/operator)
+ ops/_main.py:161: error: Unused "type: ignore" comment  [unused-ignore]

pip (https://github.com/pypa/pip)
+ src/pip/_internal/network/auth.py:233: error: Unused "type: ignore" comment  [unused-ignore]

pywin32 (https://github.com/mhammond/pywin32)
- win32/Lib/netbios.py:286:24: error: Incompatible types in assignment (expression has type "bytes", variable has type "str")  [assignment]

xarray (https://github.com/pydata/xarray)
+ xarray/core/variable.py: note: In member "data" of class "Variable":
+ xarray/core/variable.py:419: error: Incompatible override of a setter type  [override]
+ xarray/core/variable.py:419: note:  (base class "NamedArray" defined the type as "_arrayfunction[Any, Any] | _arrayapi[Any, Any]",
+ xarray/core/variable.py:419: note:  override has type "T_DuckArray | Buffer | _SupportsArray[dtype[Any]] | _NestedSequence[_SupportsArray[dtype[Any]]] | bool | int | float | complex | str | bytes | _NestedSequence[bool | int | float | complex | str | bytes]")
+ xarray/tests/test_variable.py:2707: error: Unused "type: ignore" comment  [unused-ignore]
+ xarray/tests/test_variable.py:2711: error: Unused "type: ignore" comment  [unused-ignore]
+ xarray/tests/test_datatree.py:211: error: Unused "type: ignore" comment  [unused-ignore]
+ xarray/tests/test_datatree.py:232: error: Unused "type: ignore" comment  [unused-ignore]
+ xarray/tests/test_datatree.py:269: error: Unused "type: ignore" comment  [unused-ignore]
+ xarray/tests/test_datatree.py:271: error: Unused "type: ignore" comment  [unused-ignore]
+ xarray/tests/test_datatree.py:275: error: Unused "type: ignore" comment  [unused-ignore]
+ xarray/tests/test_datatree.py:534: error: Unused "type: ignore" comment  [unused-ignore]
+ xarray/tests/test_datatree.py:1479: error: Unused "type: ignore" comment  [unused-ignore]
+ xarray/tests/test_datatree.py:1514: error: Unused "type: ignore" comment  [unused-ignore]
+ xarray/tests/test_dataset.py:6451: error: Unused "type: ignore" comment  [unused-ignore]
+ xarray/tests/test_dataset.py:6452: error: Unused "type: ignore" comment  [unused-ignore]
+ xarray/tests/test_dataset.py:6460: error: Unused "type: ignore" comment  [unused-ignore]
+ xarray/tests/test_dataset.py:6461: error: Unused "type: ignore" comment  [unused-ignore]

urllib3 (https://github.com/urllib3/urllib3)
+ src/urllib3/response.py:1259: error: Incompatible override of a setter type  [override]
+ src/urllib3/response.py:1259: note:  (base class "BaseHTTPResponse" defined the type as "str | None",
+ src/urllib3/response.py:1259: note:  override has type "str")
+ src/urllib3/response.py:1259: note:  Setter types should behave contravariantly

aiohttp-devtools (https://github.com/aio-libs/aiohttp-devtools)
+ aiohttp_devtools/runserver/serve.py:396: error: Unused "type: ignore" comment  [unused-ignore]

pandas-stubs (https://github.com/pandas-dev/pandas-stubs)
+ tests/test_frame.py:2600: error: Unused "type: ignore" comment  [unused-ignore]
+ tests/test_frame.py:2600: error: List item 0 has incompatible type "str"; expected "HashableT"  [list-item]
+ tests/test_frame.py:2600: note: Error code "list-item" not covered by "type: ignore" comment
+ tests/test_frame.py:2600: error: List item 1 has incompatible type "str"; expected "HashableT"  [list-item]
+ tests/test_frame.py:2601: error: Unused "type: ignore" comment  [unused-ignore]
+ tests/test_frame.py:2601: error: List item 0 has incompatible type "int"; expected "HashableT"  [list-item]
+ tests/test_frame.py:2601: note: Error code "list-item" not covered by "type: ignore" comment
+ tests/test_frame.py:2601: error: List item 1 has incompatible type "int"; expected "HashableT"  [list-item]
+ tests/test_frame.py:2602: error: Unused "type: ignore" comment  [unused-ignore]
+ tests/test_frame.py:2602: error: List item 0 has incompatible type "int"; expected "HashableT"  [list-item]
+ tests/test_frame.py:2602: note: Error code "list-item" not covered by "type: ignore" comment
+ tests/test_frame.py:2602: error: List item 1 has incompatible type "str"; expected "HashableT"  [list-item]
+ tests/test_frame.py:2603: error: Unused "type: ignore" comment  [unused-ignore]
+ tests/test_frame.py:2604: error: Unused "type: ignore" comment  [unused-ignore]
+ tests/test_frame.py:2605: error: Unused "type: ignore" comment  [unused-ignore]
+ tests/test_frame.py:2606: error: Unused "type: ignore" comment  [unused-ignore]

trio (https://github.com/python-trio/trio)
+ src/trio/_core/_run.py:408: error: Incompatible types in assignment (expression has type "None", variable has type "CancelStatus")  [assignment]
+ src/trio/_core/_run.py:983: error: Incompatible types in assignment (expression has type "None", variable has type "CancelStatus")  [assignment]

discord.py (https://github.com/Rapptz/discord.py)
- discord/embeds.py:179: error: Incompatible types in assignment (expression has type "int | Colour | None", variable has type "Colour | None")  [assignment]
- discord/components.py:379: error: Incompatible types in assignment (expression has type "str | Emoji | PartialEmoji | None", variable has type "PartialEmoji | None")  [assignment]

tornado (https://github.com/tornadoweb/tornado)
+ tornado/test/httpclient_test.py:889: error: Unused "type: ignore" comment  [unused-ignore]
+ tornado/test/httpclient_test.py:903: error: Unused "type: ignore" comment  [unused-ignore]

pandera (https://github.com/pandera-dev/pandera)
+ tests/core/test_schema_components.py:653: error: Unused "type: ignore" comment  [unused-ignore]
+ tests/core/test_schema_components.py:659: error: Unused "type: ignore" comment  [unused-ignore]
+ tests/core/test_schema_components.py:663: error: Unused "type: ignore" comment  [unused-ignore]

aiohttp (https://github.com/aio-libs/aiohttp)
+ aiohttp/web_fileresponse.py:163: error: Unused "type: ignore" comment  [unused-ignore]
+ aiohttp/web_fileresponse.py:164: error: Unused "type: ignore" comment  [unused-ignore]
+ aiohttp/web_fileresponse.py:394: error: Unused "type: ignore" comment  [unused-ignore]
+ aiohttp/web_fileresponse.py:395: error: Unused "type: ignore" comment  [unused-ignore]

mitmproxy (https://github.com/mitmproxy/mitmproxy)
+ mitmproxy/http.py:740: error: Unused "type: ignore" comment  [unused-ignore]
+ examples/addons/http-modify-form.py:13: error: Unused "type: ignore" comment  [unused-ignore]

kornia (https://github.com/kornia/kornia)
+ kornia/augmentation/container/augment.py:449: error: Unsupported right operand type for in ("list[DataKey] | None")  [operator]
+ kornia/augmentation/container/augment.py:450: error: Item "None" of "list[DataKey] | None" has no attribute "index"  [union-attr]
- kornia/augmentation/container/augment.py:321: error: Unused "type: ignore" comment  [unused-ignore]
- kornia/augmentation/container/augment.py:323: error: Unused "type: ignore" comment  [unused-ignore]
- kornia/augmentation/container/augment.py:443: error: Unused "type: ignore" comment  [unused-ignore]
- kornia/augmentation/container/augment.py:445: error: Unused "type: ignore" comment  [unused-ignore]

werkzeug (https://github.com/pallets/werkzeug)
+ src/werkzeug/sansio/response.py:137: error: Unused "type: ignore" comment  [unused-ignore]
+ src/werkzeug/sansio/response.py:149: error: Unused "type: ignore" comment  [unused-ignore]
+ src/werkzeug/wrappers/response.py:700: error: Unused "type: ignore" comment  [unused-ignore]
+ src/werkzeug/test.py:329: error: Unused "type: ignore" comment  [unused-ignore]
+ src/werkzeug/debug/__init__.py:303: error: Incompatible types in assignment (expression has type "None", variable has type "str")  [assignment]
- tests/test_wrappers.py:1082: error: Incompatible types in assignment (expression has type "Union[ContentRange, Any, None]", variable has type "ContentRange")  [assignment]
- tests/test_wrappers.py:1109: error: Incompatible types in assignment (expression has type "None", variable has type "ContentSecurityPolicy")  [assignment]

ilevkivskyi · 2025-01-23T00:11:52Z

OK, I think the primer looks very good. There are few new errors (for example in kornia), it looks like all of them are because we don't use binder anymore, and as I explained in PR description IMO this is correct.

The weird errors in pandas are because of this:

HashableT = TypeVar("HashableT", bound=Hashable)

class DataFrame:  # not generic in HashableT
    @property
    def columns(self) -> Index[str]: ...
    @columns.setter
    def columns(
        self, cols: AnyArrayLike | list[HashableT] | tuple[HashableT, ...]
    ) -> None: ...

What would this even mean, why not simply use Hashable? One possible way I see is to give error at definition if there are type variables not bound by self and erase them. But this would be a bunch of work for a very weird error. Any opinions? @JukkaL

ilevkivskyi · 2025-01-23T00:25:05Z

Oh, I think I understand what they want (probably):

HashableT = TypeVar("HashableT", bound=Hashable)
def foo(x: list[HashableT]) -> None: ...
def bar(x: list[Hashable]) -> None: ...

x: list[str]
foo(x)  # OK
bar(x)  # Invalid because invariance

This is a clever trick, but why not simply use Sequence? Also supporting the pattern in the above comment for this purpose would be a pain.

A5rocks · 2025-01-23T02:13:51Z

Some relevant issues (found using https://github.com/A5rocks/mypy-issues):

False positive with inherited writable property that reads subtype in subclass #14301 gets fixed
signatures of setters are not checked #12892 gets fixed
@property hides multiple type errors #11892 gets fixed

It looks like this messes up property deleters, though. Like this (taken from #17928):

class C:
    @property
    def x(self) -> int:
        return 0

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

JukkaL

Thanks for the PR! This is a long-requested feature and many users will be happy.

Looks good, just some ideas about further tests. And also please look into comments by others before merging.

JukkaL · 2025-01-23T16:33:33Z

mypy/nodes.py

@@ -1076,6 +1079,7 @@ def serialize(self) -> JsonDict:
            "name": self._name,
            "fullname": self._fullname,
            "type": None if self.type is None else self.type.serialize(),
+            "setter_type": None if self.setter_type is None else self.setter_type.serialize(),


Maybe add a serialization test case?

JukkaL · 2025-01-23T16:36:29Z

test-data/unit/check-protocols.test

+        # N: Following member(s) of "A4" have conflicts: \
+        # N:     foo: expected "B1", got "str" \
+        # N:     foo: expected setter type "C1", got "str"
+[builtins fixtures/property.pyi]


Does mypy daemon need to be modified to detect changed in setter type, i.e. does astdiff need to be updated? Maybe add a daemon test case that modifies the setter type.

JukkaL · 2025-01-23T16:44:23Z

test-data/unit/check-classes.test

@@ -8184,3 +8395,18 @@ class C:
    def f(self) -> None:
        __module__  # E: Name "__module__" is not defined
        __qualname__  # E: Name "__qualname__" is not defined
+
+[case testPropertySetterType]
+class A:


Also check a generic class with a property that uses a type var type, and a subclass that uses non-trivial generic inheritance?

ilevkivskyi added 13 commits November 10, 2024 21:13

Some initial work

5aeb5ff

Fix some tests

853431b

Fix more tests

60a6d17

Support protocols

937f115

Some incremental progress

0f65434

Merge remote-tracking branch 'upstream/master' into set-type

aa236a7

Implement actual logic for override check

30a4bba

Implement other override scenario; some fixes

26566c6

Some more fixes

96b71f5

reformat

6cb2351

more fixes

19cab84

Minor cleanup

2cb9117

More clenup, fixes, and tests

a2187de

ilevkivskyi requested review from JukkaL and hauntsaninja January 22, 2025 23:27

A5rocks mentioned this pull request Jan 23, 2025

Update type hint for the CancelStatus.parent setter python-trio/trio#3191

Open

JukkaL approved these changes Jan 23, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support properties with setter type different from getter type #18510

Support properties with setter type different from getter type #18510

ilevkivskyi commented Jan 22, 2025 •

edited

Loading

github-actions bot commented Jan 22, 2025

ilevkivskyi commented Jan 23, 2025

ilevkivskyi commented Jan 23, 2025

A5rocks commented Jan 23, 2025

JukkaL left a comment

JukkaL Jan 23, 2025

JukkaL Jan 23, 2025

JukkaL Jan 23, 2025

Support properties with setter type different from getter type #18510

Are you sure you want to change the base?

Support properties with setter type different from getter type #18510

Conversation

ilevkivskyi commented Jan 22, 2025 • edited Loading

github-actions bot commented Jan 22, 2025

ilevkivskyi commented Jan 23, 2025

ilevkivskyi commented Jan 23, 2025

A5rocks commented Jan 23, 2025

JukkaL left a comment

Choose a reason for hiding this comment

JukkaL Jan 23, 2025

Choose a reason for hiding this comment

JukkaL Jan 23, 2025

Choose a reason for hiding this comment

JukkaL Jan 23, 2025

Choose a reason for hiding this comment

ilevkivskyi commented Jan 22, 2025 •

edited

Loading