Skip to content

Conversation

@MatthewMckee4
Copy link
Contributor

Summary

Partially fixes #16953

Test Plan

Update is_subtype_of.md

Im unsure how thoroughly this needs to be tested, im not sure if it does for many more function types?

@github-actions
Copy link
Contributor

github-actions bot commented Mar 25, 2025

mypy_primer results

Changes were detected when running on open source projects
pybind11 (https://github.com/pybind/pybind11)
- error[lint:invalid-parameter-default] /tmp/mypy_primer/projects/pybind11/pybind11/setup_helpers.py:415:9: Default value of type `Literal[no_recompile]` is not assignable to annotated parameter type `(str, str, /) -> bool`
- Found 276 diagnostics
+ Found 275 diagnostics

rich (https://github.com/Textualize/rich)
- error[lint:invalid-parameter-default] /tmp/mypy_primer/projects/rich/rich/console.py:1881:9: Default value of type `Literal[currentframe]` is not assignable to annotated parameter type `() -> FrameType | None`
- Found 586 diagnostics
+ Found 585 diagnostics

black (https://github.com/psf/black)
- error[lint:invalid-return-type] /tmp/mypy_primer/projects/black/src/black/trans.py:2509:12: Object of type `Literal[is_valid_index]` is not assignable to return type `(int, /) -> bool`
- error[lint:invalid-argument-type] /tmp/mypy_primer/projects/black/src/black/lines.py:684:56: Object of type `Literal[is_import]` cannot be assigned to parameter `first_leaf_matches` of bound method `is_fmt_pass_converted`; expected type `((Leaf, /) -> bool) | None`
- Found 235 diagnostics
+ Found 233 diagnostics

@MatthewMckee4
Copy link
Contributor Author

MatthewMckee4 commented Mar 25, 2025

Ive just added the test that is failing, I'm not sure why it is failing though, any help is appreciated.

@MatthewMckee4
Copy link
Contributor Author

Maybe an interesting point, and a question i have, should:

is_subtype_of(CallableTypeFromFunction[f], CallableTypeFromFunction[g]))
=>
is_subtype_of(TypeOf[f], TypeOf[g])

I guess this just depends on how we define FunctionLiteral arms in is_subtype_of. I would think that this equality should hold, but interested to see if there are any points against this

@MichaReiser MichaReiser added the ty Multi-file analysis & type inference label Mar 25, 2025
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.

Barring the one test that should be changed, this looks good to me, and reasonably well tested!


static_assert(not is_subtype_of(TypeOf[optional_return_type], TypeOf[required_return_type]))
static_assert(is_subtype_of(TypeOf[required_return_type], TypeOf[optional_return_type]))
static_assert(is_subtype_of(TypeOf[optional_return_type], Callable[[], int | None]))
Copy link
Contributor

Choose a reason for hiding this comment

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

For good measure, we could assert this is not true the other way around.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Which one?

Copy link
Contributor

Choose a reason for hiding this comment

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

I meant both, but after reviewing the rest of the PR I think this is already adequately tested.

@carljm
Copy link
Contributor

carljm commented Mar 25, 2025

Maybe an interesting point, and a question i have, should:

is_subtype_of(CallableTypeFromFunction[f], CallableTypeFromFunction[g]))
=>
is_subtype_of(TypeOf[f], TypeOf[g])

No, this should not hold, for the same reason mentioned in my review above. TypeOf[some_function] is a singleton function literal type, which has no subtypes besides itself and Never. It is not a general callable type whose inhabitants are all functions with a substitutable signature (that's what we get from CallableTypeFromFunction[some_function]).

@MatthewMckee4
Copy link
Contributor Author

Maybe an interesting point, and a question i have, should:

is_subtype_of(CallableTypeFromFunction[f], CallableTypeFromFunction[g]))
=>
is_subtype_of(TypeOf[f], TypeOf[g])

No, this should not hold, for the same reason mentioned in my review above. TypeOf[some_function] is a singleton function literal type, which has no subtypes besides itself and Never. It is not a general callable type whose inhabitants are all functions with a substitutable signature (that's what we get from CallableTypeFromFunction[some_function]).

Okay sounds good, thank you for the response and review. I think i thought TypeOf[some_function] was more similar to CallableTypeFromFunction[some_function] than it is.

…es/is_subtype_of.md

Co-authored-by: Carl Meyer <carl@oddbird.net>
@carljm carljm enabled auto-merge (squash) March 25, 2025 22:01
@MichaReiser MichaReiser requested review from MichaReiser and removed request for MichaReiser March 25, 2025 22:01
@carljm carljm merged commit aae4d0f into astral-sh:main Mar 25, 2025
22 checks passed
@MatthewMckee4 MatthewMckee4 deleted the function-type-subtype-callable branch March 27, 2025 00:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ty Multi-file analysis & type inference

Projects

None yet

Development

Successfully merging this pull request may close these issues.

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

3 participants