Skip to content

Comments

[red-knot] Infer lambda return type as Unknown#16695

Merged
dhruvmanila merged 1 commit intomainfrom
dhruv/lambda-return-type
Mar 18, 2025
Merged

[red-knot] Infer lambda return type as Unknown#16695
dhruvmanila merged 1 commit intomainfrom
dhruv/lambda-return-type

Conversation

@dhruvmanila
Copy link
Member

@dhruvmanila dhruvmanila commented Mar 13, 2025 •
edited
Loading

Summary

Part of #15382

This PR infers the return type lambda expression as Unknown. In the future, it would be more useful to infer the expression type considering the surrounding context (#16696).

This is done by adding the expression that represents the default value of the parameters as a standalone expression. Without this, it would create cycles as described in #16547. Both function and lambda expression's parameter defaults are added as standalone expression as we cannot differentiate between the two when doing the inference in infer_parameter_definition.

Test Plan

Update existing test cases from @todo to the (verified) return type.

@dhruvmanila dhruvmanila added the ty Multi-file analysis & type inference label Mar 13, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Mar 13, 2025 •
edited
Loading

mypy_primer results

Changes were detected when running on open source projects 
pyinstrument (https://github.com/joerick/pyinstrument)
+ error[lint:call-non-callable] /tmp/mypy_primer/projects/pyinstrument/metrics/overhead.py:32:16: Object of type `() -> Unknown` is not callable
+ error[lint:call-non-callable] /tmp/mypy_primer/projects/pyinstrument/metrics/overhead.py:45:24: Object of type `() -> Unknown` is not callable
- error[lint:call-non-callable] /tmp/mypy_primer/projects/pyinstrument/pyinstrument/middleware.py:50:13: Object of type `(request) -> @Todo` is not callable
+ error[lint:call-non-callable] /tmp/mypy_primer/projects/pyinstrument/pyinstrument/middleware.py:50:13: Object of type `(request) -> Unknown` is not callable
- error[lint:call-non-callable] /tmp/mypy_primer/projects/pyinstrument/metrics/overhead.py:32:16: Object of type `() -> @Todo` is not callable
- error[lint:call-non-callable] /tmp/mypy_primer/projects/pyinstrument/metrics/overhead.py:45:24: Object of type `() -> @Todo` is not callable

pybind11 (https://github.com/pybind/pybind11)
- error[lint:call-non-callable] /tmp/mypy_primer/projects/pybind11/tests/test_virtual_functions.py:280:13: Object of type `() -> @Todo` is not callable
+ error[lint:call-non-callable] /tmp/mypy_primer/projects/pybind11/tests/test_virtual_functions.py:280:13: Object of type `() -> Unknown` is not callable

isort (https://github.com/pycqa/isort)
- error[lint:invalid-argument-type] /tmp/mypy_primer/projects/isort/isort/output.py:55:17: Object of type `(key) -> @Todo` cannot be assigned to parameter `key` of function `sort`; expected type `(str, /) -> Any | None`
+ error[lint:invalid-argument-type] /tmp/mypy_primer/projects/isort/isort/output.py:55:17: Object of type `(key) -> Unknown` cannot be assigned to parameter `key` of function `sort`; expected type `(str, /) -> Any | None`
- error[lint:invalid-argument-type] /tmp/mypy_primer/projects/isort/isort/output.py:66:17: Object of type `(key) -> @Todo` cannot be assigned to parameter `key` of function `sort`; expected type `(str, /) -> Any | None`
+ error[lint:invalid-argument-type] /tmp/mypy_primer/projects/isort/isort/output.py:66:17: Object of type `(key) -> Unknown` cannot be assigned to parameter `key` of function `sort`; expected type `(str, /) -> Any | None`
- error[lint:invalid-argument-type] /tmp/mypy_primer/projects/isort/isort/output.py:268:17: Object of type `(key) -> @Todo` cannot be assigned to parameter `key` of function `sort`; expected type `(str, /) -> Any | None`
+ error[lint:invalid-argument-type] /tmp/mypy_primer/projects/isort/isort/output.py:268:17: Object of type `(key) -> Unknown` cannot be assigned to parameter `key` of function `sort`; expected type `(str, /) -> Any | None`

@dhruvmanila dhruvmanila mentioned this pull request Mar 13, 2025
Closed
19 tasks
@dhruvmanila dhruvmanila force-pushed the dhruv/lambda-return-type branch from 51cff8e to 7a2d2bd Compare March 17, 2025 14:19
@dhruvmanila dhruvmanila marked this pull request as ready for review March 17, 2025 14:19
@AlexWaygood AlexWaygood reopened this Mar 17, 2025
@AlexWaygood AlexWaygood requested a review from dcreager as a code owner March 17, 2025 16:43
@MichaReiser MichaReiser removed their request for review March 17, 2025 16:49
@AlexWaygood AlexWaygood removed their request for review March 17, 2025 16:50
@dhruvmanila
Copy link
Member Author

dhruvmanila commented Mar 17, 2025

I'm a bit unsure about the mypy_primer failure, the logs doesn't suggest anything obvious. I plan to look at it tomorrow morning.

@AlexWaygood
Copy link
Member

AlexWaygood commented Mar 17, 2025

Is it possible that this PR is causing us to hang on one of the mypy_primer projects? I haven't checked to see if it's stopping on the same project every time

@carljm
Copy link
Contributor

carljm commented Mar 17, 2025

mypy-primer is hitting a problem on isort. I'm looking into it. It looks similar to some issues we already see on some other mypy-primer projects. I think this PR is not urgent so we can hold off until we understand the issue?

@carljm
Copy link
Contributor

carljm commented Mar 17, 2025

On looking at this a bit more, I don't think we should go forward with landing this approach. I think instead, as I mentioned in Discord, we should simply record Type::Unknown as the return type of a lambda's callable type for now.

Useful inference of a lambda's return type will require a different approach, which does the inference of the body expression based on arguments at each call site, rather than eagerly computing a return type without knowing the argument types. I don't think anything from this approach would carry over to that, so I don't think it's worth making all function defaults standalone expressions just so that we can temporarily have slightly more information about lambda return types.

@dhruvmanila
Copy link
Member Author

dhruvmanila commented Mar 18, 2025

Useful inference of a lambda's return type will require a different approach, which does the inference of the body expression based on arguments at each call site, rather than eagerly computing a return type without knowing the argument types. I don't think anything from this approach would carry over to that, so I don't think it's worth making all function defaults standalone expressions just so that we can temporarily have slightly more information about lambda return types.

Yeah, that makes sense. Thanks for looking into it, I'll update the PR to use Unknown, add a TODO.

@dhruvmanila dhruvmanila force-pushed the dhruv/lambda-return-type branch from 7a2d2bd to ce0194f Compare March 18, 2025 03:12
@dhruvmanila dhruvmanila changed the title [red-knot] Infer return type of lambda expression [red-knot] Infer lambda return type as Unknown Mar 18, 2025
@dhruvmanila dhruvmanila merged commit a69f624 into main Mar 18, 2025
23 checks passed
@dhruvmanila dhruvmanila deleted the dhruv/lambda-return-type branch March 18, 2025 17:18
dcreager added a commit that referenced this pull request Mar 18, 2025
@dcreager
Merge branch 'main' into dcreager/two-phase-binding
6cf8481 
* main:
  [playground] Avoid concurrent deployments (#16834)
  [red-knot] Infer `lambda` return type as `Unknown` (#16695)
  [red-knot] Move `name` field on parameter kind (#16830)
  [red-knot] Emit errors for more AST nodes that are invalid (or only valid in specific contexts) in type expressions (#16822)
  [playground] Use cursor for clickable elements (#16833)
  [red-knot] Deploy playground on main (#16832)
  Red Knot Playground (#12681)
  [syntax-errors] PEP 701 f-strings before Python 3.12 (#16543)
dcreager added a commit that referenced this pull request Mar 20, 2025
@dcreager
Merge branch 'dcreager/two-phase-binding' into dcreager/legacy-typeva…
96f261e 
…r-instance

* dcreager/two-phase-binding: (210 commits)
  Update docs
  clippy
  Fix docs
  with_self instead of push/pop_self
  Update crates/red_knot_python_semantic/src/types/call/arguments.rs
  lint
  Report conflicting forms for call as a whole, not for any signature
  Add conflicting form test case
  Use option<usize>
  Remove some unneeded clones
  evaluate_known_cases
  Infer types when constructing CallArgumentTypes
  Remove old comment
  Use VecDeques for arguments and types
  Fix tests
  Add comment
  Add some comments
  Use parameter_types for all special cases
  [playground] Avoid concurrent deployments (#16834)
  [red-knot] Infer `lambda` return type as `Unknown` (#16695)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@carljm carljm carljm approved these changes

@sharkdp sharkdp Awaiting requested review from sharkdp

@dcreager dcreager Awaiting requested review from dcreager

Assignees

No one assigned

Labels

ty Multi-file analysis & type inference

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@dhruvmanila @AlexWaygood @carljm