Skip to content

Comments

[red-knot] Add callable subtyping for callable instances and bound methods#17105

Merged
carljm merged 8 commits intoastral-sh:mainfrom
MatthewMckee4:bounded-method-subtype
Apr 1, 2025
Merged

[red-knot] Add callable subtyping for callable instances and bound methods#17105
carljm merged 8 commits intoastral-sh:mainfrom
MatthewMckee4:bounded-method-subtype

Conversation

@MatthewMckee4
Copy link
Contributor

@MatthewMckee4 MatthewMckee4 commented Mar 31, 2025

Summary

Trying to improve #17005
Partially fixes #16953

Test Plan

Update is_subtype_of.md

@github-actions
Copy link
Contributor

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

mypy_primer results

Changes were detected when running on open source projects 
rich (https://github.com/Textualize/rich)
- error[lint:invalid-argument-type] /tmp/mypy_primer/projects/rich/tests/test_progress.py:292:60: Object of type `MockClock` cannot be assigned to parameter `get_time` of function `track`; expected type `(() -> int | float) | None`
- Found 578 diagnostics
+ Found 577 diagnostics

black (https://github.com/psf/black)
- error[lint:invalid-argument-type] /tmp/mypy_primer/projects/black/src/black/__init__.py:1280:48: Object of type `<bound method `readline` of `BytesIO`> | @Todo(instance attribute on class with dynamic base)` cannot be assigned to parameter 1 (`readline`) of function `detect_encoding`; expected type `() -> bytes | bytearray`
- error[lint:invalid-argument-type] /tmp/mypy_primer/projects/black/src/black/__init__.py:1280:48: Object of type `<bound method `readline` of `BytesIO`> | @Todo(instance attribute on class with dynamic base)` cannot be assigned to parameter 1 (`readline`) of function `detect_encoding`; expected type `() -> bytes | bytearray`
- error[lint:invalid-argument-type] /tmp/mypy_primer/projects/black/src/black/__init__.py:1280:48: Object of type `<bound method `readline` of `BytesIO`> | @Todo(instance attribute on class with dynamic base)` cannot be assigned to parameter 1 (`readline`) of function `detect_encoding`; expected type `() -> bytes | bytearray`
- Found 212 diagnostics
+ Found 209 diagnostics

@AlexWaygood AlexWaygood added the ty Multi-file analysis & type inference label Mar 31, 2025
MatthewMckee4
MatthewMckee4 commented Apr 1, 2025
View reviewed changes
crates/red_knot_python_semantic/src/types.rs Outdated
Comment on lines 4335 to 4341
let signature = self.function(db).signature(db).clone();
let parameters = Parameters::new(signature.parameters().iter().skip(1).cloned());

Type::Callable(CallableType::General(GeneralCallableType::new(
db,
Signature::new(parameters, signature.return_ty),
)))
Copy link
Contributor Author

@MatthewMckee4 MatthewMckee4 Apr 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure this is how we will want to implement this , but i think its right to not check the name of the variable, and since staticmethod is not supported, this should be fine, but when that is implemented i think we will have to check decorators

@MatthewMckee4 MatthewMckee4 marked this pull request as ready for review April 1, 2025 00:20
MichaReiser
MichaReiser reviewed Apr 1, 2025
View reviewed changes
@MichaReiser
Copy link
Member

MichaReiser commented Apr 1, 2025 •
edited
Loading

Not sure, but this might be a good one for @dcreager to review?

carljm
carljm approved these changes Apr 1, 2025
View reviewed changes
Copy link
Contributor

@carljm carljm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks reasonable to me! The tests are good, and we can always change the implementation of BoundMethodType::into_callable_type if we decide we need to preserve the first argument.

Thank you!

@MatthewMckee4 @carljm
Update is_subtype_of.md
fbf94d0 
Co-authored-by: Carl Meyer <[email protected]>
dcreager
dcreager approved these changes Apr 1, 2025
View reviewed changes
@carljm carljm changed the title [red-knot] Add callable subtyping for bounded methods [red-knot] Add callable subtyping for callable instances and bound methods Apr 1, 2025
carljm
carljm reviewed Apr 1, 2025
View reviewed changes
@carljm carljm enabled auto-merge (squash) April 1, 2025 23:39
@carljm carljm merged commit eb3e176 into astral-sh:main Apr 1, 2025
22 checks passed
@MatthewMckee4 MatthewMckee4 deleted the bounded-method-subtype branch April 1, 2025 23:49
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

@MichaReiser MichaReiser MichaReiser left review comments

@dcreager dcreager dcreager approved these changes

@AlexWaygood AlexWaygood Awaiting requested review from AlexWaygood

@sharkdp sharkdp Awaiting requested review from sharkdp

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.

[red-knot] Implement subtyping/assignability between different callable objects

5 participants

@MatthewMckee4 @MichaReiser @dcreager @carljm @AlexWaygood