Skip to content
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

Make type completeness results consistent for different platforms #2691

Merged
merged 6 commits into from
Jul 10, 2023

Conversation

TeamSpen210
Copy link
Contributor

If the type completeness test is run on Windows, the slightly different imports produce different counts, and also cause the JSON file to end up with symbols in different orders. This sorts the JSON file to prevent that from affecting diff results, and configures pyright to always behave like the platform is Linux. Perhaps it would be better to test both platforms and merge the results (so OS-specific code gets checked for missing types), but that would require a bigger change to the script.

@codecov
Copy link

codecov bot commented Jul 7, 2023

Codecov Report

Merging #2691 (75dca8f) into master (ed975f8) will not change coverage.
The diff coverage is n/a.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #2691   +/-   ##
=======================================
  Coverage   98.84%   98.84%           
=======================================
  Files         113      113           
  Lines       16474    16474           
  Branches     3004     3004           
=======================================
  Hits        16283    16283           
  Misses        134      134           
  Partials       57       57           

@TeamSpen210
Copy link
Contributor Author

Maybe this might not work, there's still 4 symbols that don't match...

@jakkdl
Copy link
Member

jakkdl commented Jul 7, 2023

Ooh, this is great.

If you want to hunt down what's the cause of the difference you can [temporarily] print the ones with known type to the json and not just the ones with diagnostics, but I think we can just remove withKnownType from the results and ignore it in the check if you don't manage to figure it out. We don't actually care about the raw number of known types after all, and the actual goal is to bring withUnknownType to 0.

@TeamSpen210
Copy link
Contributor Author

The difference are these symbols:

  • trio._path.Path.hardlink_to()
  • trio._path.Path.is_relative_to()
  • trio._path.Path.readlink()
  • trio._path.Path.with_stem()

... Which are caused by me using Python 3.11 not 3.8.

@TeamSpen210 TeamSpen210 requested a review from jakkdl July 8, 2023 00:13
Copy link
Contributor

@A5rocks A5rocks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me.

Copy link
Member

@jakkdl jakkdl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks awesome, thanks for the fix.

@jakkdl jakkdl merged commit 6d71047 into python-trio:master Jul 10, 2023
@TeamSpen210 TeamSpen210 deleted the type-completeness branch July 12, 2023 02:19
@A5rocks A5rocks added the typing Adding static types to trio's interface label Jul 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
typing Adding static types to trio's interface Windows
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants