[ty] Preserve Self for class-object intersection receivers#25704
[ty] Preserve Self for class-object intersection receivers#25704charliermarsh wants to merge 25 commits into
Conversation
Typing conformance resultsNo changes detected ✅Current numbersThe percentage of diagnostics emitted that were expected errors held steady at 92.23%. The percentage of expected errors that received a diagnostic held steady at 87.42%. The number of fully passing files held steady at 92/134. |
Memory usage reportSummary
Significant changesClick to expand detailed breakdownprefect
sphinx
trio
flake8
|
|
| Lint rule | Added | Removed | Changed |
|---|---|---|---|
invalid-argument-type |
0 | 3 | 0 |
invalid-return-type |
1 | 1 | 0 |
| Total | 1 | 4 | 0 |
Raw diff:
Tanjun (https://github.com/FasterSpeeding/Tanjun)
- tanjun/annotations.py:1449:31 error[invalid-argument-type] Argument to `Choices.__init__` is incorrect: Argument type `_EnumT@__getitem__` does not satisfy constraints (`int`, `int | float`, `str`) of type variable `_ChoiceT`
- tanjun/annotations.py:1454:31 error[invalid-argument-type] Argument to `Choices.__init__` is incorrect: Argument type `_EnumT@__getitem__` does not satisfy constraints (`int`, `int | float`, `str`) of type variable `_ChoiceT`
- tanjun/annotations.py:1459:31 error[invalid-argument-type] Argument to `Choices.__init__` is incorrect: Argument type `_EnumT@__getitem__` does not satisfy constraints (`int`, `int | float`, `str`) of type variable `_ChoiceT`
hydpy (https://github.com/hydpy-dev/hydpy)
- hydpy/core/masktools.py:39:16 error[invalid-return-type] Return type does not match returned value: expected `Self@array2mask`, found `ndarray[tuple[Any, ...], Unknown]`
prefect (https://github.com/PrefectHQ/prefect)
+ src/prefect/input/run_input.py:900:16 error[invalid-return-type] Return type does not match returned value: expected `GetAutomaticInputHandler[T@receive_input] | GetInputHandler[R@receive_input]`, found `GetInputHandler[R@receive_input & ~Top[AutomaticRunInput[Unknown]]]`337915b to
0453acf
Compare
fed3eae to
cf7cc04
Compare
cf7cc04 to
79d22a4
Compare
Merging this PR will not alter performance
Comparing 
Footnotes
|
sharkdp
left a comment
There was a problem hiding this comment.
Sorry, still not a full review yet. I'm struggling with some of the concepts here.
| ## Classmethods preserve unrelated occurrences of `Self` | ||
|
|
||
| ```py | ||
| from __future__ import annotations | ||
|
|
||
| from typing import Self, cast | ||
| from ty_extensions import Intersection | ||
|
|
||
| class Factory[T]: | ||
| @classmethod | ||
| def identity(cls, value: object) -> T: | ||
| return cast(T, value) | ||
|
|
||
| class Owner: | ||
| def inspect( | ||
| self: Self, | ||
| cls: Intersection[type[Factory[Self]], type[Owner]], | ||
| ) -> None: | ||
| reveal_type(cls.identity(self)) # revealed: Self@inspect | ||
|
|
||
| class ConcreteOwner(Owner): ... | ||
| class Both(Factory[ConcreteOwner], Owner): ... | ||
|
|
||
| ConcreteOwner().inspect(Both) | ||
| ``` |
There was a problem hiding this comment.
This test passes on main. Can we describe more clearly what is being tested here? Is this a regression test for something that came up during the implementation? Do we need to make sure that the Self@inspect doesn't turn into something else?
| When `T` is inferred as an intersection, `type[T]` preserves constraints on the runtime class. | ||
| Nominal class constraints survive, value and structural refinements do not, and `TypedDict`, | ||
| `NewType`, and class-object constraints project to their concrete runtime classes. |
There was a problem hiding this comment.
For me, these introductory prose sections are often the most valuable part of an mdtest, but I have trouble understanding this. The text here also seemingly ignores the existing tests in this section.
When
Tis inferred as an intersection,type[T]preserves constraints on the runtime class.
What kinds of constraints?
Nominal class constraints survive, value and structural refinements do not
Are we talking about negative intersection elements? What are value and structural refinements?
Is it: ~B survives as ~type[B], but ~Literal[1] does not survive in the type -> metatype transformation?
class-object constraints project to their concrete runtime classes
?
| def _(x: int): | ||
| if x != 1: | ||
| reveal_type(runtime_type(x)) # revealed: type[int] | ||
| if runtime_type(x) is int: |
There was a problem hiding this comment.
Why is this here? This doesn't affect the type of x and doesn't seem related to what we want to test?
| def _(x: Intersection[A, Not[B]]): | ||
| reveal_type(runtime_type(x)) # revealed: type[A] & ~type[B] |
There was a problem hiding this comment.
Hm, this is interesting. On your branch,runtime_type transforms A & B into type[A] & type[B]. However, for any other covariant generic class, if we have a function
def lift[T](x: T) -> Covariant[T]: ...we would transform A & B into Covariant[A & B], not Covariant[A] & Covariant[B]. Now, Covariant[A & B] is a subtype of Covariant[A] & Covariant[B], but they're not equivalent. Is type an exception?
There was a problem hiding this comment.
The typing spec explicitly defines type[A | B] as syntactic sugar for type[A] | type[B]. Since the "type argument" of type cannot be an arbitrary type, but must be a nominal class or typevar (otherwise type isn't really meaningful), type[A | B] would not be valid or defined, if it were not explicitly defined as equivalent to type[A] | type[B].
I haven't spent a long time thinking about it, but I think the same thing applies to type[A & B]. It is only valid/meaningful if it is explicitly defined as shorthand for type[A] & type[B].
sharkdp
left a comment
There was a problem hiding this comment.
I'm very sorry that I only have a high level review comment here, but I spent some time staring at this PR (and its predecessor) today and it feels like we're actually trying to solve a pretty fundamental problem (calling a method on an intersection type), but the implementation looks like a collection of special cases in various random places of the code base. It's certainly possible that all of this is needed (I will take another look tomorrow morning when I'm less tired), but it feels a bit off to me.
Some things that I was thinking about while reading this:
- Is
type[C]the only type that needs separate special cases, or will we need more patches for other types that are also not handled by the original PR? - Why are there changes in the generics solver in this PR?
- Could/should some of this be solved by eagerly simplifying
type[A] & type[B]totype[A & B]?
|
|
||
| ### Classmethod binding skips negative metaclass constraints | ||
|
|
||
| A negative metaclass constraint describes the class object's runtime class, not instances of the |
There was a problem hiding this comment.
There are too many meanings of "constraint" already. It would be great if we could keep the "negative element of an intersection type" terminology.
|
Let's just close this then. There are a significant number of bugs and limitations related to intersection types but I don't think it makes sense to prioritize this work right now if it's going to take a bunch of time. |
Summary
When a classmethod is accessed through an intersection such as
Intersection[type[Factory], type[Extra]], we currently find the member on one element and bindSelffrom that element alone. This loses the rest of the class-object intersection:The intersected class object represents a class whose instances satisfy both constraints, so binding
Selfmust preserve the corresponding intersection of instance types.This builds on #25819, which addressed the post-merge review of #25626.