-
Notifications
You must be signed in to change notification settings - Fork 235
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
Initial check-in of type specification conformance suite. #1552
Conversation
…ance issues currently covered by tests.
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.
Thank you for working 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.
Was reading through many of the test cases, had one question come up
… for some reason.
Merging, as discussed. Feedback on the structure or initial set of tests still very welcome :-) |
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 for putting this together! It's a great start.
I have a few small comments that I'll send a PR to fix when I get a chance.
One bigger area that doesn't seem ideal is that we're checking in the exact text of each type checker's output.
An idea for an alternative approach:
- In test cases, we write a magic comment "# E" or "# E: explanation for why this is an error" at every place where there should be an error
- For each type checker, we parse the output to extract the list of line numbers
- Then we compare the set of lines where there should be an error with the set where the type checker actually emitted an error.
This should remove some of the manual work in updating the .toml files.
@@ -0,0 +1,47 @@ | |||
# Byte-compiled / optimized / DLL files |
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 could probably go in a global .gitignore instead.
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.
One bigger area that doesn't seem ideal is that we're checking in the exact text of each type checker's output.
I agree it's not ideal, but I don't see a good alternative. Type checkers report the same error on different lines. Unless we mandate exactly where an error is reported (which is probably not viable), there will be many differences. Plus, type checkers sometimes report additional errors or warnings unrelated to the functionality being tested. I don't see a way around "manual scoring". That's why I think it's important to keep each test file relatively small and targeted.
|
||
## Motivation | ||
|
||
[PEP 729](https://peps.python.org/pep-0729/) provides a structured and documented way to specify and evolve the Python type system. In support of this effort, an official [Python typing spec](https://github.com/python/typing/tree/main/docs/spec) has been drafted. This spec consolidates details from various historical typing-related PEPs. The spec will be modified over time to clarify unspecified and under-specified parts of the type system. It will also be extended to cover new features of the type system. |
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.
Link to the rendered version: https://typing.readthedocs.io/en/latest/spec/index.html
@@ -0,0 +1,7 @@ | |||
tomli |
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 read earlier that we require 3.12, so shouldn't we be able to use the stdlib's tomllib module instead?
f.write(pyre_config) | ||
|
||
return True | ||
except: |
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.
Avoid raw except
Sorry for taking so long to look at this! It looks good to me, with the caveat that I didn't carefully go through all of the individual tests and conformance results - just read a couple and skimmed the rest. I'll see what I can do to speed up pytype. EDIT: I misunderstood what PytypeTypeChecker.run_tests is doing; it's not running pytype multiple times. But it is running both type checking and type inference, so I'm going to see if we can get some savings by only doing checking. |
This is a first cut at a conformance test suite for the typing specification. It builds on earlier work done by @hauntsaninja. The test suite is still very incomplete, but it provides a framework that should allow us to rapidly build out the missing tests.