-
-
Notifications
You must be signed in to change notification settings - Fork 2.9k
Consistently store settable property type #18774
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
Consistently store settable property type #18774
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for fixing! Confirmed that this fixes the original crash 🙏
Nice, looks like a lot of the aiohttp primer is from the |
Change in The couple new errors in |
OK, I actually checked the |
Diff from mypy_primer, showing the effect of this PR on open source code: aiohttp (https://github.com/aio-libs/aiohttp)
+ aiohttp/abc.py:84: error: Unused "type: ignore" comment [unused-ignore]
+ aiohttp/web_routedef.py:57: error: Unused "type: ignore" comment [unused-ignore]
+ aiohttp/web_routedef.py:82: error: Unused "type: ignore" comment [unused-ignore]
+ aiohttp/helpers.py:148: error: Unused "type: ignore" comment [unused-ignore]
+ aiohttp/helpers.py:177: error: Unused "type: ignore" comment [unused-ignore]
+ aiohttp/helpers.py:248: error: Unused "type: ignore" comment [unused-ignore]
+ aiohttp/helpers.py:873: error: Unused "type: ignore" comment [unused-ignore]
+ aiohttp/helpers.py:890: error: Unused "type: ignore" comment [unused-ignore]
+ aiohttp/client_reqrep.py:325: error: Unused "type: ignore" comment [unused-ignore]
+ aiohttp/client.py:1250: error: Unused "type: ignore" comment [unused-ignore]
+ aiohttp/client.py:1287: error: Unused "type: ignore" comment [unused-ignore]
+ aiohttp/web_server.py:43: error: Unused "type: ignore" comment [unused-ignore]
+ aiohttp/web_server.py:52: error: Unused "type: ignore" comment [unused-ignore]
+ aiohttp/web_app.py:167: error: Unused "type: ignore" comment [unused-ignore]
+ aiohttp/web_app.py:182: error: Unused "type: ignore" comment [unused-ignore]
+ aiohttp/web_app.py:216: error: Unused "type: ignore" comment [unused-ignore]
+ aiohttp/web_runner.py:257: error: Unused "type: ignore" comment [unused-ignore]
+ aiohttp/worker.py:203: error: Unused "type: ignore" comment [unused-ignore]
+ aiohttp/test_utils.py:160: error: Unused "type: ignore" comment [unused-ignore]
+ aiohttp/test_utils.py:270: error: Unused "type: ignore" comment [unused-ignore]
+ aiohttp/test_utils.py:278: error: Unused "type: ignore" comment [unused-ignore]
+ aiohttp/pytest_plugin.py:45: error: Unused "type: ignore" comment [unused-ignore]
+ aiohttp/pytest_plugin.py:53: error: Unused "type: ignore" comment [unused-ignore]
+ aiohttp/pytest_plugin.py:402: error: Unused "type: ignore" comment [unused-ignore]
+ aiohttp/pytest_plugin.py:409: error: Unused "type: ignore" comment [unused-ignore]
xarray (https://github.com/pydata/xarray)
+ xarray/core/variable.py: note: In function "encoding":
+ xarray/core/variable.py:857: error: Incompatible types in assignment (expression has type "dict[Never, Never]", variable has type "None") [assignment]
+ xarray/core/variable.py:858: error: Incompatible return value type (got "None", expected "dict[Any, Any]") [return-value]
trio (https://github.com/python-trio/trio)
+ src/trio/_file_io.py:431: error: Unused "type: ignore[misc]" comment [unused-ignore]
+ src/trio/_path.py:186: error: Unused "type: ignore[misc]" comment [unused-ignore]
|
This change seems to cause another crash unfortunately. Noticed this first for Home Assistant with # pip install qbittorrent-api==2024.2.59
from qbittorrentapi.transfer import Transfer Was able to reproduce it with from functools import wraps
def transfer_speed_limits_mode(): ...
def transfer_set_speed_limits_mode(): ...
class Transfer:
@property
@wraps(transfer_speed_limits_mode)
def speed_limits_mode(self) -> str:
...
@speed_limits_mode.setter
@wraps(transfer_set_speed_limits_mode)
def speed_limits_mode(self, val: bool) -> None:
... $ mypy --show-traceback --no-incremental test.py
test.py:9: error: Missing return statement [empty-body]
test.py:7: error: INTERNAL ERROR -- Please try using mypy master on GitHub:
https://mypy.readthedocs.io/en/stable/common_issues.html#using-a-development-mypy-build
Please report a bug at https://github.com/python/mypy/issues
version: 1.16.0+dev.af5186e1bccdc3983a289bea962bb940ef6857f8
Traceback (most recent call last):
File "mypy/checker.py", line 576, in accept
stmt.accept(self)
File "mypy/nodes.py", line 580, in accept
return visitor.visit_overloaded_func_def(self)
File "mypy/checker.py", line 641, in visit_overloaded_func_def
self._visit_overloaded_func_def(defn)
File "mypy/checker.py", line 711, in _visit_overloaded_func_def
assert isinstance(var_type, CallableType)
AssertionError:
test.py:7: : note: use --pdb to drop into pdb Do note though that I was only able to reproduce it with a compiled build
|
OK, fair enough, I just need to do the same fallback dance I do for the setter (and I can actually make it precise as we only care about the return type). |
Follow up for #18774 Fix for crash is trivial, properly handle getter the same way as setter. Note I also consistently handle callable instances.
Fixes #18764
There are two things important to understand this PR:
Var.type
of getter (as a decorator), and also in overallOverloadedFuncDef.type
. The latter is ugly, but unfortunately it is hard to get rid of, since some code in multiple plugins rely on this.It turns out there are three inconsistencies in how these two things interact (first one causes the actual crash):
semanal.py
we only store it the second way (i.e. asOverloadedFuncDef.type
).Var.type
).Essentially I simply remove these inconsistencies.