Further improvements to functools.partial handling #17425
Conversation
- Fixes another crash case / type inference in that case - Fix a false positive when calling the partially applied function - TypeTraverse / comment / daemon test follow up ilevkivskyi mentioned on the original PR
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅ |
| @@ -229,9 +229,11 @@ def partial_new_callback(ctx: mypy.plugin.FunctionContext) -> Type: | |||
| partial_names.append(fn_type.arg_names[i]) | |||
| elif actuals: | |||
There was a problem hiding this comment.
I think this can be simply else, since if the above branch is false this is always true.
| kind = actual_arg_kinds[actuals[0]] | ||
| if kind == ArgKind.ARG_NAMED: | ||
| if kind == ArgKind.ARG_NAMED or kind == ArgKind.ARG_STAR2: |
There was a problem hiding this comment.
I know this works at runtime, but is this a common pattern that we should allow? I mean do people often use partial(f, x=1)(x=2) etc?
There was a problem hiding this comment.
I have at least one such use in a checked codebase.
| kind = actual_arg_kinds[actuals[0]] | ||
| if kind == ArgKind.ARG_NAMED: | ||
| if kind == ArgKind.ARG_NAMED or kind == ArgKind.ARG_STAR2: | ||
| kind = ArgKind.ARG_NAMED_OPT | ||
| partial_kinds.append(kind) |
There was a problem hiding this comment.
What if kind == ArgKind.ARG_STAR? I mean in a situation like this
def foo(x: int, y: int) -> None: ...
xs: list[int]
partial(foo, *xs)IIUC this will create a callable with two star-args in signature, because star actual will map to both formals. I am not sure what is the best way to handle this (each way I can think of may create false positives), maybe we can at least handle the case where corresponding arg_type is fixed length tuple. But we definitely should not create invalid callables with multiple star-args.
| seen_args.add(a) | ||
| actual_args.append(a) | ||
| actual_arg_kinds.append(ctx.arg_kinds[i][j]) | ||
| actual_arg_names.append(ctx.arg_names[i][j]) |
There was a problem hiding this comment.
Looking at this it seems to me a better strategy for the call site may be using get_attribute_hook() for __call__? Unfortunately this hook is not called in is_subtype() etc yet. But at least it will be possible to precisely type-check something like this in future
def foo(fn: Callable[[int, str], int]) -> None: ...
fn = partial(some_other_fn, 1, 2)
foo(fn)| bad: Bad | ||
| partial(f_more, **kwargs)(**bad) # E: Argument "y" to "f_more" has incompatible type "str"; expected "int" | ||
| good: Good | ||
| partial(f_more, **kwargs)(**good) |
There was a problem hiding this comment.
You are deleting this, but it looks like you are not testing a similar situation. All your tests are (effectively) about partial(f, x=1)(x=2), while this one specifically checks for (IMO more realistic/common) use case equivalent to partial(f, x=1)(y=2) (non-overlapping arguments that "complete" the call).
See also #17423