[ty] Check overloaded function implementation consistency#17800
[ty] Check overloaded function implementation consistency#17800LaBatata101 wants to merge 2 commits intoastral-sh:mainfrom
Conversation
|
|
The second "issue" looks like correct behavior to me. A missing annotation is equivalent to an annotation of |
Ah okay, I was checking with |
|
I don't know off the top of my head why the |
7cb6a4a to
393c508
Compare
|
@LaBatata101 I went ahead and rebased your PR passed the Red Knot renaming change. Make sure you pull (force) before trying to push new changes. |
393c508 to
a17f71b
Compare
Thanks! |
I don't think so. I don't think the use of diff --git a/crates/ty_python_semantic/src/types/infer.rs b/crates/ty_python_semantic/src/types/infer.rs
index 543d8ea060..8fc98276c9 100644
--- a/crates/ty_python_semantic/src/types/infer.rs
+++ b/crates/ty_python_semantic/src/types/infer.rs
@@ -1251,6 +1251,13 @@ impl<'db> TypeInferenceBuilder<'db> {
for (idx, overloaded_signature) in overloaded_signatures
.iter()
.filter(|overloaded_signature| {
+ eprintln!(
+ "{} -- {}",
+ impl_signature.display(self.db()),
+ overloaded_signature.display(self.db())
+ );
+ dbg!(impl_signature.is_parameters_assignable_to(self.db(), overloaded_signature));
+ dbg!(overloaded_signature.is_return_type_assignable_to(self.db(), impl_signature));
!(impl_signature.is_parameters_assignable_to(self.db(), overloaded_signature)
&& overloaded_signature.is_return_type_assignable_to(self.db(), impl_signature))
})And got this output on a failing test: What this shows is that we are getting accurate signatures off of all the callables involved (and once we have a And I can reproduce this behavior without involving So I think the next step here is to debug that minimal test case and fix it in |
I will look into that this week |
|
I don't think the issue is in the logic of |
|
I don't think there should be any specialization happening here? These functions are unspecialized generics (as seen by the fact that the signatures printed out in my debugging above still have You're right that it looks like the issue is specific to typevars; it also looks like it's specific to a bound typevar: https://play.ty.dev/7bca2116-a4f4-4e07-a518-b1077403aec4 And it looks like it's specific to comparing Ok, I just figured out what I think it is. Pushed the fix in #17901 Rebasing this PR on top of that should fix the dataclass-transform tests, I think. |
a17f71b to
9216d72
Compare
9216d72 to
c4405e8
Compare
|
So, I've pulled the new changes made in #17910, but this case still failing @overload
def func() -> None: ...
@overload
def func[T](x: T) -> T: ...
def func[T](x: T | None = None) -> T | None:
return xI've printed the output of |
|
Ugh, sorry, this is astral-sh/ty#95 -- it impacts the PEP 695 version but not the legacy-typevar version, because in this case we consider them different typevars. I'm a little hesitant to land this overload consistency check without fixing that bug first, since it will cause more false positives than we currently do. Sorry... any chance you're interested in trying to tackle that bug? |
I can try, but I don't understand much about type theory or the python type system, do you have any resources that I can take a look at? Much of the stuff you guys talk about goes over my head. |
|
Probably the most useful written resource I can recommend (and this isn't saying a great deal) is the typing spec, particularly starting with the "core concepts" section. But that issue may not be the simplest place to start. Hopefully one of us can get to it before long, because I would like to land this PR -- your work here looks great. |
|
Thanks for your contribution. I'll close this as it seems to have gone stale. Anyone interested in picking this up. Feel free to open a new PR which addresses the feedback. |
This PR is just waiting for #18634 to get merged. I've been a little busy the last couple of days, but I'll start working on #18634 again soon. |
|
Based on the fact that #18634 is now closed, I'm going to close this. Feel free to comment here if you think this should be reopened. |
Summary
I've separated the implementation of
is_assignable_tointois_parameters_assignable_toandis_return_type_assignable_toas discussed in Discord, that solved the problem I was having. But, there are still a few issues that I need some help with.The first one is, some tests are failing in
dataclass_transform.md, specifically when the function is annotated withdataclass_transformlike this one:I guess this is related to this part of the spec:
Not sure how these transforms should be handled.
The second issue I have is that in this python code:My implementation is not creating the diagnostic for the function implementation when it's missing the return type.I'll add more tests after I solve these issues.
Closes astral-sh/ty#109
Test Plan
Markddown tests.