-
-
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
Infer ParamSpec constraint from arguments #15896
Infer ParamSpec constraint from arguments #15896
Conversation
This comment has been minimized.
This comment has been minimized.
OK, I have tried to tweak the special-casing after looking at the |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
To summarize on the
IMO this is a clear net positive. |
@JukkaL Could you please also check this one? I remember you were interested in this issue. |
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, this fixes several ParamSpec use cases, some of which seem pretty fundamental. I think that the special casings are justified and can't think of a more general approach.
reveal_type(register(lambda x: f(x), x=1)) # N: Revealed type is "def (x: Any)" | ||
register(lambda x: f(x)) # E: Missing positional argument "x" in call to "register" | ||
register(lambda x: f(x), y=1) # E: Unexpected keyword argument "y" for "register" | ||
reveal_type(register(lambda x: f(x), x=1)) # N: Revealed type is "def (x: Literal[1]?)" |
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.
Does this work with other kinds of arguments, such as positional, or more than one argument? Maybe also test these, unless they are covered elsewhere.
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.
I think positional should work, and multiple args too. In general, for "first order" calls like apply(fn, arg1, arg2)
practically everything should work. However, as I mentioned in the PR description for "higher order" calls like apply(apply, fn, arg1, ag2)
some combinations of formal/actual kinds are tricky to support, and they will not work with this PR.
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.
(to clarify, those "higher order" calls currently don't work either, and this PR will partially fix it, in the sense that inferred types, will be correct, but mypy will complain about subtyping because of the inferred argument kinds)
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.
Hm, actually I think I found a small tweak in subtyping that may allow more "higher order" calls with a slight reduction in safety (that only affects imprecise inferred kinds). Let me try it.
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.
No, it looks too arbitrary. Let's wait a bit more and see which cases people will complain about. After we will gather more data, we can tune relaxed subtyping for callables with imprecise_arg_kinds=True
.
Diff from mypy_primer, showing the effect of this PR on open source code: anyio (https://github.com/agronholm/anyio)
+ src/anyio/from_thread.py:395: error: Argument 1 to "submit" of "Executor" has incompatible type "Callable[[Callable[..., Awaitable[T_Retval]], VarArg(object), DefaultNamedArg(str, 'backend'), DefaultNamedArg(dict[str, Any] | None, 'backend_options')], T_Retval]"; expected "Callable[[Callable[[], Coroutine[Any, Any, None]], str, dict[str, Any] | None], T_Retval]" [arg-type]
+ src/anyio/from_thread.py:396: error: Unused "type: ignore" comment [unused-ignore]
Expression (https://github.com/cognitedata/Expression)
- tests/test_fn.py:15: error: Argument 1 to "TailCall" has incompatible type "int"; expected <nothing> [arg-type]
- tests/test_fn.py:15: error: Argument 2 to "TailCall" has incompatible type "int"; expected <nothing> [arg-type]
steam.py (https://github.com/Gobot1234/steam.py)
- steam/ext/commands/commands.py:401: error: Argument 2 to "maybe_coroutine" has incompatible type "Context[Any]"; expected "MC" [arg-type]
+ steam/ext/commands/commands.py:401: error: Argument 1 to "maybe_coroutine" has incompatible type "CheckReturnType"; expected "Callable[[Context[Any]], MC | Awaitable[MC]]" [arg-type]
+ steam/ext/commands/commands.py:401: note: "CheckReturnType.__call__" has type "Callable[[MC], MC]"
- steam/ext/commands/bot.py:474: error: Argument 2 to "maybe_coroutine" has incompatible type "Context[Any]"; expected "MC" [arg-type]
+ steam/ext/commands/bot.py:474: error: Argument 1 to "maybe_coroutine" has incompatible type "CheckReturnType"; expected "Callable[[Context[Any]], MC | Awaitable[MC]]" [arg-type]
+ steam/ext/commands/bot.py:474: note: "CheckReturnType.__call__" has type "Callable[[MC], MC]"
prefect (https://github.com/PrefectHQ/prefect)
+ src/prefect/engine.py:1131: error: Argument 1 to "create_call" has incompatible type "function"; expected "Callable[[Task[Any, Any], FlowRunContext, dict[str, Any], Iterable[PrefectFuture[Any, Any]] | None, Literal['future', 'state', 'result'], BaseTaskRunner | None], <nothing>]" [arg-type]
- src/prefect/engine.py:1131: error: Argument 1 to "create_call" has incompatible type "function"; expected "Callable[[VarArg(<nothing>), KwArg(<nothing>)], <nothing>]" [arg-type]
- src/prefect/engine.py:1132: error: Argument "task" to "create_call" has incompatible type "Task[Any, Any]"; expected <nothing> [arg-type]
- src/prefect/engine.py:1134: error: Argument "parameters" to "create_call" has incompatible type "dict[str, Any]"; expected <nothing> [arg-type]
- src/prefect/engine.py:1135: error: Argument "wait_for" to "create_call" has incompatible type "Iterable[PrefectFuture[Any, Any]] | None"; expected <nothing> [arg-type]
- src/prefect/engine.py:1136: error: Argument "return_type" to "create_call" has incompatible type "Literal['future', 'state', 'result']"; expected <nothing> [arg-type]
- src/prefect/engine.py:1137: error: Argument "task_runner" to "create_call" has incompatible type "BaseTaskRunner | None"; expected <nothing> [arg-type]
pytest (https://github.com/pytest-dev/pytest)
+ src/_pytest/_py/path.py:758: error: Argument 1 has incompatible type overloaded function; expected "Callable[[str, Any, Any], TextIOWrapper]" [arg-type]
+ src/_pytest/_py/path.py:1270: error: Argument 1 has incompatible type overloaded function; expected "Callable[[str], str]" [arg-type]
discord.py (https://github.com/Rapptz/discord.py)
- discord/ext/commands/core.py:1279: error: Argument 2 to "maybe_coroutine" has incompatible type "Context[BotT]"; expected "Context[BotT]" [arg-type]
- discord/ext/commands/hybrid.py:415: error: Argument 2 to "maybe_coroutine" has incompatible type "Context[Bot]"; expected "Context[BotT]" [arg-type]
pydantic (https://github.com/samuelcolvin/pydantic)
- pydantic/v1/class_validators.py:148: error: Need type annotation for "f_cls" [var-annotated]
|
Fixes #12278
Fixes #13191 (more tricky nested use cases with optional/keyword args still don't work, but they are quite tricky to fix and may selectively fixed later)
This unfortunately requires some special-casing, here is its summary:
Callable[P, T]
is non-generic and non-lambda, do not put it into inference second pass.P
without using arguments mapped to*args: P.args
etc., do not add the constraint forP
vs those arguments (this applies to both top-level callable constraints, and for nested callable constraints against callables that are known to have imprecise argument kinds).(Btw TODO I added is not related to this PR, I just noticed something obviously wrong)