-
Notifications
You must be signed in to change notification settings - Fork 263
Add the Zuban Type Checker #2067
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
base: main
Are you sure you want to change the base?
Conversation
I just signed https://www.python.org/psf/contrib/contrib-form/, I hope that will fix CI eventually. |
@@ -116,6 +116,6 @@ class ClassC(TypedDict): | |||
TA1: TypeAlias = Annotated[int | str, ""] | |||
TA2 = Annotated[Literal[1, 2], ""] | |||
|
|||
T = TypeVar("T") | |||
T = TypeVar("T") # E?: T is already defined |
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 think we should just remove this line, it looks like an oversight that the TypeVar was redefined.
Type checkers probably should error on this line, but it's not the subject of this test case so better to leave it out.
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 removed it.
Thanks for adding this! I looked over the test file changes and they mostly look good, as noted above. I didn't review all the results for Zuban. One concern: I know you're using a partly paid model, where some users have to pay for usage of the type checker. How does that system interact with usage of Zuban in this repo? For example, could we run into issues if the conformance suite grows past some line count, or if we tried to run all type checkers in CI? |
conformance/src/type_checker.py
Outdated
|
||
# Run "mypy --version" to ensure that it's installed and to work | ||
# around timing issues caused by malware scanners on some systems. | ||
self.get_version() |
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.
Is this needed for zuban? This was a hack that was added for mypy specifically (see comment). If it is needed, please update this comment. If it's not needed, it can be deleted.
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.
Good catch, I removed it. I guess I didn't proof-read my code well enough.
Thanks for adding this. I didn't do a line-by-line review, but I did skim through it. I have a few questions:
|
No, I'm most likely going to open source it anyway in the near future and at the moment there the line count limits are disabled.
It is based on Mypy's test suite, so if you look at the automatic test results and open them side by side you quite often see the exact same results. If the results are not the same the issues are sometimes still the same even if there is an additional error, it still stems from the same fact (e.g. some sort of thing is not supported, because it's not covered by Mypy's test suite). I also specifically fixed the issues that would differ from Mypy's results to be more similar to Mypy, so I wouldn't have to write my own Notes. I'm lazy in a weird way :)
I will add notes. |
I'm concerned about adding support for a type checker with a proprietary license. I'm volunteering my (and my company's) time for work on typing, and the contract has always been that the software we're directly supporting is licensed freely. For example, while we're directly supporting pyright, which is MIT licensed, we're not directly supporting the proprietary pylance language server. (Of course, we're indirectly supporting proprietary software and improving typing as a whole and fixing bugs reported by developers and users of proprietary software, but that benefits all typing users.) Frankly speaking, if I'm supposed to support proprietary software, me or my company expects to be paid for it. So, I'm -1 on merging this (and any similar) PRs. |
@erictraut @JelleZijlstra I think I fixed the remaining issues. @srittau Did you read the part about open sourcing? |
If Zuban would go Open Source, my concerns would obviously be irrelevant. |
Does anybody know why the CLA part of CI is still failing? I signed it yesterday. |
I hoped that closing and reopening would fix the CLA issue but no dice. @ambv could you help take a look?
That's good to know! @srittau I broadly agree with this concern (and I probably wouldn't support adding Zuban to typeshed CI while it's still proprietary), but I think it's not too bad for the conformance suite, where we don't have to do that much more than run the type checker and record the results. |
It looks like the CLA form you signed requires manual processing, which probably hasn't gone through yet. The other way to sign the CLA is by clicking the button in the bot comment / visiting https://cla.python.org/, which is a fully automated process |
Fair enough, especially since I personally stay away from the conformance tests. |
@brianschubert Thanks that went through. |
I have added the type checker that I have recently written. The results should be mostly the same, since it's mostly using the same tests.
I have only changed a few things:
conformance = "Pass"
for unknown conformance if the automated tests pass. This was useful while adding the type checker and will probably be useful for other type checkers as well.I would be happy if this was merged soon, because the merge conflicts can be kind of annoying for this one.