-
Notifications
You must be signed in to change notification settings - Fork 1.6k
[ty] Add and test when constraint sets are satisfied by their typevars #21129
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Diagnostic diff on typing conformance testsNo changes detected when running ty on typing conformance tests ✅ |
|
| def bounded[T: Base](): | ||
| static_assert(ConstraintSet.always().satisfied_by_all_typevars(T)) | ||
| static_assert(not ConstraintSet.never().satisfied_by_all_typevars(T)) | ||
| static_assert(ConstraintSet.range(Never, T, Super).satisfied_by_all_typevars(T)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think I understand why T <= Super satisfies T <= Base, given that Super isn't a subtype of Base. Or am I reading this the wrong way around? Or is the Never part important?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We discussed this sync — the key point is that the implication is in the other direction. (The typevar bounds/constraints need to imply the constraint set being checked.) So here, T must specialize to a subtype of Base. Base is a subtype of Super, so every valid specialization of T is also a subtype of Super. (Technically, this test is in the inferable section, so we only need one valid specialization of T to satisfy the constraint set. In this case, every valid specialization does, so you'll see that this same test also holds down in the non-inferable section.)
| static_assert(ConstraintSet.range(Never, T, Base).satisfied_by_all_typevars(T)) | ||
| static_assert(ConstraintSet.range(Never, T, Sub).satisfied_by_all_typevars(T)) | ||
|
|
||
| # This succeeds because T can specialize to Never |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think I understand the why but I trust you on this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is trying to show that Never is sneaky, especially for inferable typevars. Since T is inferable, we only need one specialization to satisfy the constraint set. Never is a valid specialization, since Never ≤ Base. And Never ≤ Unrelated, so the constraint set is satisfied for the T = Never case. That's enough for an inferable typevar.
The corresponding test in the non-inferable section fails, though, since there we need all valid specializations to satisfy the constraint set. And T = Base is a counter-example where it doesn't.
This tells me that it might help to change the structure of this file? Right now I have inferable/non-inferable as the top-level sections, and different kinds of typevar bounds/constraints as (unlabeled) subsections. Maybe instead I should have "unbounded", "bound", and "constrained" as the top-level sections, to more clearly call out how the behavior is different for inferable vs non-inferable?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This tells me that it might help to change the structure of this file?
I did this, and added more explanatory comments for each example. Hopefully that helps clarify the logic a bit.
| static_assert(ConstraintSet.always().satisfied_by_all_typevars(T)) | ||
| static_assert(not ConstraintSet.never().satisfied_by_all_typevars(T)) | ||
| static_assert(ConstraintSet.range(Never, T, Unrelated).satisfied_by_all_typevars(T)) | ||
| static_assert(ConstraintSet.range(Never, T, Super).satisfied_by_all_typevars(T)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, same here. Why is Super satisfying the constraint, given that it isn't Base or Unrelated?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as above. This is in inferable position, so we only need one specialization of T to satisfy the constraint set. T = Base does, so the check passes. Down below in the non-inferable section, the check fails, because T = Unrelated is a valid specialization that doesn't satisfy the constraint set.
crates/ty_python_semantic/resources/mdtest/type_properties/satisfied_by_all_typevars.md
Outdated
Show resolved
Hide resolved
* origin/main: (21 commits) [ty] Update "constraint implication" relation to work on constraints between two typevars (#21068) [`flake8-type-checking`] Fix `TC003` false positive with `future-annotations` (#21125) [ty] Fix lookup of `__new__` on instances (#21147) Fix syntax error false positive on nested alternative patterns (#21104) [`pyupgrade`] Fix false positive for `TypeVar` with default on Python <3.13 (`UP046`,`UP047`) (#21045) [ty] Reachability and narrowing for enum methods (#21130) [ty] Use `range` instead of custom `IntIterable` (#21138) [`ruff`] Add support for additional eager conversion patterns (`RUF065`) (#20657) [`ruff-ecosystem`] Fix CLI crash on Python 3.14 (#21092) [ty] Infer type of `self` for decorated methods and properties (#21123) [`flake8-bandit`] Fix correct example for `S308` (#21128) [ty] Dont provide goto definition for definitions which are not reexported in builtins (#21127) [`airflow`] warning `airflow....DAG.create_dagrun` has been removed (`AIR301`) (#21093) [ty] follow the breaking API changes made in salsa-rs/salsa#1015 (#21117) [ty] Rename `Type::into_nominal_instance` (#21124) [ty] Filter out "unimported" from the current module [ty] Add evaluation test for auto-import including symbols in current module [ty] Refactor `ty_ide` completion tests [ty] Render `import <...>` in completions when "label details" isn't supported [`refurb`] Preserve digit separators in `Decimal` constructor (`FURB157`) (#20588) ...
| When a typevar is in an inferable position, the constraint set only needs to be satisfied for _some_ | ||
| valid specialization. The most common inferable position occurs when invoking a generic function: | ||
| all of the function's typevars are inferable, because we want to use the argument types to infer | ||
| which specialization is being invoked. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is great. Thanks for adding it
MichaReiser
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks again for taking the time to explain this to me during our phone call. The reasoning in the mdtests make sense to me. Altough I'm still finding the DSL slightly confusing. But maybe that just is because I fail to map it to an example using Python
crates/ty_python_semantic/resources/mdtest/type_properties/satisfied_by_all_typevars.md
Outdated
Show resolved
Hide resolved
| static_assert(not ConstraintSet.range(Never, T, Unrelated).satisfied_by_all_typevars(inferable=tuple[()])) | ||
|
|
||
| # (T = Base) is a valid specialization, which satisfies (T ≤ Super). | ||
| static_assert(ConstraintSet.range(Never, T, Super).satisfied_by_all_typevars(inferable=tuple[T])) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm still somewhat confused by the DSL here but I (maybe?) finally figured out how to read this?
Is my understanding correct that this results in:
Never <= any(Base, Unrelated) <= Super
which is true because Base satisfies this constraint.
And the next example is:
Never < forall(Base, Unrelated) < Super
which is false, because Unrelated doesn't satisfy this constraint.
The part I'm struggling with right now is what a real-world example of static_assert(ConstraintSet.range(Never, T, Super).satisfied_by_all_typevars(inferable=tuple[T])) would look like?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm still somewhat confused by the DSL here but I (maybe?) finally figured out how to read this?
I would formalize it differently, but I think your notation leads to the right understanding:
Is my understanding correct that this results in:
Never <= any(Base, Unrelated) <= Superwhich is true because
Basesatisfies this constraint.
Yes
And the next example is:
Never < forall(Base, Unrelated) < Superwhich is false, because
Unrelateddoesn't satisfy this constraint.
This should use <= instead of < like the first example, but otherwise yes.
The part I'm struggling with right now is what a real-world example of
static_assert(ConstraintSet.range(Never, T, Super).satisfied_by_all_typevars(inferable=tuple[T]))would look like?
I'm actually not sure what Python I could write that would result in this check either! We need something that wants to check that an instance of T is assignable to Super. The non-inferable case is easy:
def f[T: (Base, Unrelated)](t: T):
x: Super = tBut in the T inferable case, we would be invoking this function. And I'm not sure what Python code would lead to a T ≤ Super check.
The problem is that I can't limit myself to implementing these algorithms for constraint set checks that have obvious Python analogues. 😅
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But in the
Tinferable case, we would be invoking this function. And I'm not sure what Python code would lead to aT ≤ Supercheck.
Ah maybe via the return type:
def f[T: (Base, Unrelated)]() -> T:
raise NotImplementedError
x: Super = f()(Disregard the fact that there is no real function body that we could write that would satisfy that signature!)
* origin/main: [ty] Fix generic inference for non-dataclass inheriting from generic dataclass (#21159) Update etcetera to 0.11.0 (#21160) Fix missing diagnostics for notebooks (#21156) [ty] Fix tests for definition completions (#21153) Bump v0.14.3 (#21152) [ty] Don't provide completions when in class or function definition (#21146) [ty] Use the top materialization of classes for narrowing in class-patterns for `match` statements (#21150)
This PR adds a new
satisfied_by_all_typevarmethod, which implements one of the final steps of actually using these dang constraint sets. Constraint sets exist to help us check assignability and subtyping of types in the presence of typevars. We construct a constraint set describing the conditions under which assignability holds between the two types. Then we check whether that constraint set is satisfied for the valid specializations of the relevant typevars (which is this new method).We also add a new
ty_extensions.ConstraintSetmethod so that we can test this method's behavior in mdtests, before hooking it up to the rest of the specialization inference machinery.