Let the multiple inheritance checks consider callable objects as possible subtypes of usual functions (Fixes #14852). #14855
Conversation
…ible subtypes of usual functions (Fixes python#14852). The solution is inspired by the `visit_instance` method of `SubtypeVisitor`. The `testMultipleInheritanceOverridingOfFunctionsWithCallableInstances` tests the new functionality for decorated and non-decorated functions and callable objects.
|
According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉 |
| class F: | ||
| def __call__(self, x: int) -> None: ... | ||
|
|
||
| def dec2(f: Callable[[Any, int], None]) -> F: ... |
There was a problem hiding this comment.
maybe give it a more descriptive name, e.g. make_callable?
There was a problem hiding this comment.
Do you mean make_callable1 and make_callable2 for dec1 and dec2? Or different names, like make_callable and modify_function or so?
There was a problem hiding this comment.
Yeah, no strong opinion TBH. After I looked at this test for a few minutes, I got acquainted with "dec1", "dec2", "A", "B" etc. It's just that on first glance dec1 and dec2 seemed less than descriptive, but they'd be perfect names in Go :)
There was a problem hiding this comment.
Never worked with Go. Maybe I should try it...
There was a problem hiding this comment.
(https://go.dev/talks/2014/names.slide does a good job at describing what's considered good style there)
There was a problem hiding this comment.
i instead of index. If I remember correctly, pylint complains if I don't write at least idx.
| if isinstance(first_type, Instance): | ||
| call = find_member("__call__", first_type, first_type, is_operator=True) | ||
| if call and isinstance(second_type, FunctionLike): | ||
| second_sig = self.bind_and_map_method(second, second_type, ctx, base2) | ||
| ok = is_subtype(call, second_sig, ignore_pos_arg_names=True) |
There was a problem hiding this comment.
🤔 I keep wondering if this could be somehow generalized, e.g. check compatibility between any callables the same way, with simple functions being "upgraded" to a synthetic Instance with a __call__ member. I didn't check the feasibility of this too much, though.
There was a problem hiding this comment.
Yes, there are possibly more places where similar checks are performed (or should be performed).
A quick search for find_member("__call__",: five hits that seem to serve the same or similar purposes besides my own one.
However, there seem to be many small differences. So, refactoring it might not be simple.
There was a problem hiding this comment.
I had a second look, just searching for "__call__". 51 hits. Many special cases. I have no idea how far a generalisation like the one you suggested would be helpful. And it would be too much effort for me, currently.
There was a problem hiding this comment.
Yeah, totally understood. BTW I'm just trying to help with reviews, I'm not a maintainer.
|
@hauntsaninja: seems like we need a maintainer's review. Would you like to? Or who else to ask? |
…ons (#14855) Let the multiple inheritance checks consider callable objects as possible subtypes of usual functions (Fixes #14852). The solution is inspired by the `visit_instance` method of `SubtypeVisitor`. The `testMultipleInheritanceOverridingOfFunctionsWithCallableInstances` tests the new functionality for decorated and non-decorated functions and callable objects.
Let the multiple inheritance checks consider callable objects as possible subtypes of usual functions (Fixes #14852).
The solution is inspired by the
visit_instancemethod ofSubtypeVisitor. ThetestMultipleInheritanceOverridingOfFunctionsWithCallableInstancestests the new functionality for decorated and non-decorated functions and callable objects.