-
-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
Use type variable bound when it appears as actual during inference #16178
Conversation
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.
Nice! I think this makes sense, and the primer hits for Argument Clinic and aioredis both look like clear improvements. (Haven't looked too deeply at the hydra-zen hits.)
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.
Nice, since you're looking at this, can I also interest you in #13220 ? I tried a couple different ways to fix that one and all caused problems.
@AlexWaygood I might be missing something, but aioredis one looks like a regression, no? Repro:
from typing import TypeVar, Union, Iterable
KeyT = Union[bytes, str]
_KeyT = TypeVar("_KeyT", bound=KeyT)
def simpler(keys: _KeyT):
iter(keys) # now has type error
def lora(keys: Union[_KeyT, Iterable[_KeyT]]):
iter(keys) # now has type error
Oh shoot, I misread the diff — I thought it was saying that the |
@hauntsaninja That is an unrelated issue. I however already have a fix locally that should likely not cause any adverse side effects. I can make a PR now. Also FWIW I want to focus now on finishing the |
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.
Thanks, this looks good. I added regression test.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Diff from mypy_primer, showing the effect of this PR on open source code: CPython (Argument Clinic) (https://github.com/python/cpython)
+ Tools/clinic/clinic.py:3114: error: Unused "type: ignore" comment [unused-ignore]
hydra-zen (https://github.com/mit-ll-responsible-ai/hydra-zen)
- src/hydra_zen/typing/_builds_overloads.py:129: error: Overloaded function signatures 5 and 9 overlap with incompatible return types [overload-overlap]
- src/hydra_zen/typing/_builds_overloads.py:150: error: Overloaded function signatures 6 and 7 overlap with incompatible return types [overload-overlap]
- src/hydra_zen/typing/_builds_overloads.py:172: error: Overloaded function signatures 7 and 9 overlap with incompatible return types [overload-overlap]
- src/hydra_zen/typing/_builds_overloads.py:373: error: Overloaded function signatures 6 and 10 overlap with incompatible return types [overload-overlap]
- src/hydra_zen/typing/_builds_overloads.py:394: error: Overloaded function signatures 7 and 8 overlap with incompatible return types [overload-overlap]
- src/hydra_zen/typing/_builds_overloads.py:416: error: Overloaded function signatures 8 and 10 overlap with incompatible return types [overload-overlap]
- src/hydra_zen/typing/_builds_overloads.py:539: error: Overloaded function signatures 2 and 5 overlap with incompatible return types [overload-overlap]
- src/hydra_zen/structured_configs/_implementations.py:919: error: Overloaded function signatures 5 and 9 overlap with incompatible return types [overload-overlap]
- src/hydra_zen/structured_configs/_implementations.py:941: error: Overloaded function signatures 6 and 7 overlap with incompatible return types [overload-overlap]
- src/hydra_zen/structured_configs/_implementations.py:963: error: Overloaded function signatures 7 and 9 overlap with incompatible return types [overload-overlap]
|
This should help with re-enabling the use of
ParamSpec
infunctools.wraps
(as it looks like some of the new errors in AlexWaygood#11 are caused by not handling this).