-
Notifications
You must be signed in to change notification settings - Fork 1.7k
[ty] infer function's return type #17371
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
base: main
Are you sure you want to change the base?
Conversation
|
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
|
If a function contains a yield expression, i.e. if it is a generator, current type inference is incorrect. But supporting this requires solving another issues, so I will leave it as a TODO for now. As for return type inference for normal functions, I think the work is completed. |
3ba99d1 to
35f4a75
Compare
|
Sorry that I haven't gotten around to reviewing this PR yet. There's just been a lot to do, and providing this feature is lower on my priority list than some other features. But I realize that places a burden on you to keep it up to date with (rapidly changing) main branch. But I will try to find time to review it soon. |
0a16b46 to
126aef8
Compare
91c66c1 to
22e8594
Compare
22e8594 to
391ee91
Compare
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.
Partial review here; plane is landing now so submitting the comments I have. Will come back to this later.
Thank you for working on this, and sorry for the slow review! It's a very useful feature.
crates/ty_python_semantic/resources/mdtest/function/return_type.md
Outdated
Show resolved
Hide resolved
crates/ty_python_semantic/resources/mdtest/function/return_type.md
Outdated
Show resolved
Hide resolved
|
It appears that abnormal memory consumption was occurring during the scipy inspection and ty was killed. The profiling result seem to suggest that |
|
Hmm, in fact, this is a result of fixed-point iterations not converging and the intersection type growing without bound? |
|
The handling of unions in |
|
mypy_primer: #17371 (comment) |
Yes, it could be problematic, but it's required to keep set-theoretic types in disjunctive normal form, which is valuable, and in most practical cases, unions/intersections don't grow large enough for it to be a problem. We could set some size-maximums with fallback to a less precise type, would have to look at the specific case that causes a blowup. If it's due to fixpoint iteration not converging, then that seems like the key issue to fix. |
|
Fixed |
|
Opened #22055. Thanks to this change, scipy no longer seems to be killed. |
|
| Lint rule | Added | Removed | Changed |
|---|---|---|---|
unsupported-operator |
35,575 | 0 | 73 |
possibly-missing-attribute |
15,046 | 86 | 339 |
invalid-argument-type |
3,481 | 82 | 239 |
non-subscriptable |
2,222 | 0 | 0 |
invalid-assignment |
1,638 | 11 | 33 |
not-iterable |
1,341 | 0 | 8 |
unresolved-attribute |
1,209 | 12 | 3 |
no-matching-overload |
728 | 0 | 0 |
call-non-callable |
703 | 2 | 0 |
invalid-return-type |
363 | 2 | 26 |
too-many-positional-arguments |
290 | 3 | 0 |
unknown-argument |
146 | 8 | 0 |
unsupported-base |
145 | 0 | 0 |
invalid-await |
115 | 0 | 6 |
invalid-key |
115 | 0 | 0 |
unused-ignore-comment |
20 | 83 | 0 |
missing-argument |
44 | 4 | 0 |
index-out-of-bounds |
43 | 4 | 0 |
parameter-already-assigned |
2 | 19 | 0 |
division-by-zero |
8 | 0 | 0 |
invalid-context-manager |
7 | 0 | 0 |
deprecated |
3 | 2 | 0 |
invalid-type-form |
2 | 0 | 0 |
possibly-unresolved-reference |
0 | 2 | 0 |
invalid-exception-caught |
1 | 0 | 0 |
invalid-method-override |
1 | 0 | 0 |
| Total | 63,248 | 320 | 727 |
|
Given the many new diagnostics, I think we should consider adding a new I also suggest running the |
800bfc8 to
0a80f98
Compare
| if self | ||
| .signature_type | ||
| .as_bound_method() | ||
| .is_none_or(|method| !method.is_init(self.db)) |
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.
Summary
This PR closes astral-sh/ty#128.
FunctionType::infer_return_typeis added to infer the return type of a function when its return type is not specified.TODOs
- [ ] infer generator's return type- [ ] infer coroutine's return type- [ ] infer lambda function's return typeTest Plan
New test cases are added to
mdtest/function/return_type.md.