-
-
Notifications
You must be signed in to change notification settings - Fork 292
Add some typing to astroid.inference
#1415
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
Conversation
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.
LGTM, thank you @DanielNoord
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.
Regarding the mypy bug. The issue is probably more general. object.__new__
is defined correctly in typeshed, but the decorator is just not applied to the class like it's the case with functions. An example
from typing import Any, Callable
def change_class_to_int(cls: type[object]) -> int:
return 1
def change_return_to_int(func: Callable[..., Any]) -> int:
return 2
@change_class_to_int
class A:
...
@change_return_to_int
def func() -> None:
...
reveal_type(A)
reveal_type(func)
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.
It might make sense to update the PR title, I think.
@@ -862,14 +862,15 @@ def _do_compare( | |||
return util.Uninferable | |||
# (or both, but "True | False" is basically the same) | |||
|
|||
assert retval is not None | |||
return retval # it was all the same value |
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.
Can you add a type hint to the definition of retval
in L840?
util.Uninferable
astroid.inference
astroid/inference.py
Outdated
@@ -837,7 +843,7 @@ def _do_compare( | |||
>>> _do_compare([1, 3], '<=', [2, 4]) | |||
util.Uninferable | |||
""" | |||
retval = None | |||
retval: Union[Literal[None], bool, Type[util.Uninferable]] = None |
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.
retval: Union[Literal[None], bool, Type[util.Uninferable]] = None | |
retval: Union[None, bool] = None |
Literal[None]
is not necessary. Both mypy and pyright understand None
.
Additionally, Uninferable
is never assigned to retval
, only returned directly.
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.
Hmm there a quite a bit of Literal[None]
's in the codebase. Perhaps we should update those at some point.
Co-authored-by: Marc Mueller <[email protected]>
Steps
Description
The typing on this is incorrect I believe.
https://github.com/PyCQA/astroid/blob/c0d2e5c89a1525e9f500c43b04529628a70e070f/astroid/util.py#L33-L34
I don't think
mypy
recognises that the decorator onUninferable
makesutil.Uninferable
an instance instead of the type. I checked the issue tracker and there is no issue about this yet, so I am slightly doubting whether I am in correct in this, but I think this is the correct fix.Edit: Seeing as
pyright
does get this right I opened python/mypy#12265.Type of Changes
Related Issue