New type inference: complete transitive closure by ilevkivskyi · Pull Request #15754 · python/mypy (original) (raw)

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 andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation7 Commits16 Checks0 Files changed

Conversation

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})

This is a first follow-up for #15287 (I like how my PR titles sound like research paper titles, LOL)

This PR completes the new type inference foundations by switching to a complete and well founded algorithm [1] for transitive closure (that replaces more ad hoc initial algorithm that covered 80% of cases and was good for experimenting with new inference scheme). In particular the algorithm in this PR covers two important edge cases (see tests). Some comments:

• I don't intend to switch the default for --new-type-inference, I just want to see the effect of the switch on mypy_primer, I will switch back to false before merging
• This flag is still not ready to be publicly announced, I am going to make another 2-3 PRs from the list in Foundations for non-linear solver and polymorphic application #15287 before making this public.
• I am not adding yet the unit tests as discussed in previous PR. This PR is already quite big, and the next one (support for upper bounds and values) should be much smaller. I am going to add unit tests only for transitive_closure() which is the core of new logic.
• While working on this I fixed couple bugs exposed in TypeVarTuple support: one is rare technical corner case, another one is serious, template and actual where swapped during constraint inference, effectively causing outer/return context to be completely ignored for instances.
• It is better to review the PR with "ignore whitespace" option turned on (there is big chunk in solve.py that is just change of indentation).
• There is one questionable design choice I am making in this PR, I am adding extra_tvars as an attribute of Constraint class, while it logically should not be attributed to any individual constraint, but rather to the full list of constrains. However, doing this properly would require changing the return type of infer_constrains() and all related functions, which would be a really big refactoring.

[1] Definition 7.1 in https://inria.hal.science/inria-00073205/document

Btw I guess I should quote the thesis where I took the algorithm I am using here. I will update the PR description.

This comment has been minimized.

OK, I went through mypy_primer and I am generally happy, most of the output matches the previous PR. There are two differences:

• A bunch of changes in Expression: these are caused by extensive use of ParamSpec and Concatenate, which are not yet supported (and are planned in fourth PR)
• A bunch of overlapping overload errors are caused by exposing an unrelated bug in detach_callable() (which is arguably a hack btw). It uses some heuristic to figure out what are the current class type variables instead of just calling self.scope.active_class(), and this causes it to create some bizarre functions like def [T] () -> def [T] (T) -> T which accidentally used to work before. I am going to push a fix for this now.

This comment has been minimized.

All looks good, I will now switch the default back to false to prepare for merge.

Diff from mypy_primer, showing the effect of this PR on open source code:

hydra-zen (https://github.com/mit-ll-responsible-ai/hydra-zen)

• src/hydra_zen/typing/_builds_overloads.py:44: error: Overloaded function signatures 1 and 2 overlap with incompatible return types [misc]
• src/hydra_zen/typing/_builds_overloads.py:44: error: Overloaded function signatures 1 and 9 overlap with incompatible return types [misc]
• src/hydra_zen/typing/_builds_overloads.py:270: error: Overloaded function signatures 1 and 3 overlap with incompatible return types [misc]
• src/hydra_zen/typing/_builds_overloads.py:270: error: Overloaded function signatures 1 and 10 overlap with incompatible return types [misc]
• src/hydra_zen/typing/_builds_overloads.py:561: error: Overloaded function signatures 3 and 4 overlap with incompatible return types [misc]
• src/hydra_zen/typing/_builds_overloads.py:561: error: Overloaded function signatures 3 and 7 overlap with incompatible return types [misc]
• src/hydra_zen/structured_configs/_implementations.py:834: error: Overloaded function signatures 1 and 2 overlap with incompatible return types [misc]
• src/hydra_zen/structured_configs/_implementations.py:834: error: Overloaded function signatures 1 and 9 overlap with incompatible return types [misc]

OK, it looks like no one is around to take a look at this. I have the next PR ready, so I am going to merge this tomorrow, unless there are objections.

ilevkivskyi added a commit that referenced this pull request

Aug 9, 2023

This is a third PR in series following #15287 and #15754. This one is quite simple: I just add basic support for polymorphic inference involving type variables with upper bounds and values. A complete support would be quite complicated, and it will be a corner case to already rare situation. Finally, it is written in a way that is easy to tune in the future.

I also use this PR to add some unit tests for all three PRs so far, other two PRs only added integration tests (and I clean up existing unit tests as well).

1 participant