- Notifications
You must be signed in to change notification settings - Fork3.8k
Fix stubtest warnings, run stubtest on CI#2999
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
remove changes since this is addressed in PRpygame#2999
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
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 haven't tested it myself but as far as I can tell it looks good to me.
Comparing this PR to 2.1.2 freetype module this now produces a typing error: Is this an intentional change because of: from the docs? Everything else looks good in my review. Tests pass locally, (except a mixer init one that I think is possibly just a bad test or and SDL_mixer bug as you don't seem to actually be able to always set the available number of channels anymore). |
Thanks for the reviews! Yes, freetype is not auto-imported by pygame, it must be explicitly imported at runtime as you point out. So it's aight if the typechecker thinks freetype is not a part of pygame here when it's not explicitly imported. Since it's a bit of a large PR, I'd like to wait for one or two more reviews. From what I know, zoldalma is in the process of giving this an elaborate review ATM, so let's not merge this PR in just yet |
Sorry@ankith26 looks like the new vector functions have given you a conflict here |
No worries, I'll resolve it with a rebase after zolds review is done (he's taking many days to review, yes 😆 ) |
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.
Well, I wanted to go trough it in more detail (and it still took me longer than I wanted it to take), but left some comments on what should be changed/considered in my opinion.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
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 the changes, LGTM! 🎉
Uh oh!
There was an error while loading.Please reload this page.
This PR fixes many issues with our typestubs, and also adds a
mypy.stubtesttest program on CI to test for issues in typestubs.Firstly this PR moves typestubs from
buildconfig/pygame-stubstobuildconfig/stubs/pygamebecause stubtest needs to run in a directory where pygame stubs are inpygamesubdir.Some of our util typestubs in
_commonhas a leading underscore, which leads to pycharm not recognising those (I heard from someone, I did not test this myself), so I fixed this, and it still works withmypyafter the changesI added some missing stubs, some in
_sdl2and also stubs forbaseandrwobject.Another change I did was to properly type
VectorElementwiseProxyas a generic, so now it works well with bothVector2andVector3types, and correctly errors at incorrectelementwise()calls (like mixing vector2 and vector3 elementwise)Apart from that, there are a lot more fixes in this PR, so I believe this PR fixes most of the typestub issues, but there might still be issues left, that are not caught by mypy stubtest. A good future enhancement would be running typestub tests on our examples and maybe some dedicated unit tests for this, but this PR is already too large, so this is something for later.
I've tried to keep code changes to a minimum in this PR, but there are a few places where I had to make some code changes, for instance, I replaced
clock_initwithclock_newfollowing the PR#3001 becauseClock.__init__leads to some stub issues, and also according to python docs fortp_newSo clock being immutable, this change makes sense