Skip to content

Conversation

MatthewMckee4
Copy link
Contributor

Summary

Part of astral-sh/ty#129

There were previously some false positives here.

Test Plan

Updated is_subtype_of.md and is_assignable_to.md

Copy link
Contributor

github-actions bot commented Jun 29, 2025

mypy_primer results

Changes were detected when running on open source projects
async-utils (https://github.com/mikeshardmind/async-utils)
- TOTAL MEMORY USAGE: ~49MB
+ TOTAL MEMORY USAGE: ~45MB

alerta (https://github.com/alerta/alerta)
- TOTAL MEMORY USAGE: ~117MB
+ TOTAL MEMORY USAGE: ~106MB

nox (https://github.com/wntrblm/nox)
-     memo fields = ~66MB
+     memo fields = ~72MB

mkosi (https://github.com/systemd/mkosi)
- TOTAL MEMORY USAGE: ~117MB
+ TOTAL MEMORY USAGE: ~129MB
-     memo fields = ~97MB
+     memo fields = ~106MB

bandersnatch (https://github.com/pypa/bandersnatch)
- TOTAL MEMORY USAGE: ~80MB
+ TOTAL MEMORY USAGE: ~88MB

paasta (https://github.com/yelp/paasta)
-     memo fields = ~156MB
+     memo fields = ~171MB

prefect (https://github.com/PrefectHQ/prefect)
+ error[invalid-argument-type] src/prefect/tasks.py:779:13: Argument to bound method `__init__` is incorrect: Expected `int | float | list[int | float] | ((int, /) -> list[int | float]) | None`, found `int | float | list[int | float] | (((int, /) -> list[int | float]) & ~<class 'NotSet'>) | (type[NotSet] & ~<class 'NotSet'>) | Unknown`
- Found 4186 diagnostics
+ Found 4187 diagnostics

scikit-learn (https://github.com/scikit-learn/scikit-learn)
- TOTAL MEMORY USAGE: ~717MB
+ TOTAL MEMORY USAGE: ~652MB

@MatthewMckee4 MatthewMckee4 marked this pull request as ready for review June 29, 2025 15:02
@MatthewMckee4
Copy link
Contributor Author

MatthewMckee4 commented Jun 29, 2025

I think the mypy primer is correct.

Since NotSet is an empty class and therefore not a subclass of float | int | list[float] | Callable[[int], list[float]]).

Maybe a bit of an unfortunate diagnostic but it seems sound.

Their mistake is not marking the class with @final right?

@ntBre ntBre added the ty Multi-file analysis & type inference label Jun 29, 2025
@MatthewMckee4 MatthewMckee4 changed the title Add subtyping between SubclassOf and CallableType [ty] Add subtyping between SubclassOf and CallableType Jun 29, 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.

This looks good to me, and I ran the property tests at a million iterations with no failures.

@carljm
Copy link
Contributor

carljm commented Jul 3, 2025

Oh, and I agree with your primer evaluation 100%! x is not NotSet should not narrow type[NotSet] out of a union unless NotSet is a final class. And we do handle it correctly if it's final: https://play.ty.dev/75aa3432-c763-4cb2-be00-e2297b386748

@carljm
Copy link
Contributor

carljm commented Jul 3, 2025

The reason this accurate diagnostic was previously hidden is because type in typeshed has a very forgiving gradual __call__ definition: def __call__(self, *args: Any, **kwds: Any) -> Any: .... Our fallback here is just to see if type is assignable, and that gradual __call__ definition makes an arbitrary instance of type assignable to any callable.

@carljm carljm merged commit 352b896 into astral-sh:main Jul 3, 2025
37 checks passed
@MatthewMckee4 MatthewMckee4 deleted the subclass-of-callable-has-relation-to branch July 16, 2025 21:12
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.

3 participants