Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

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

Merged
zoldalma999 merged 6 commits intomainfromankith26-stubtest
Feb 26, 2022
Merged

Conversation

@ankith26
Copy link
Contributor

@ankith26ankith26 commentedJan 22, 2022
edited
Loading

This PR fixes many issues with our typestubs, and also adds amypy.stubtest test program on CI to test for issues in typestubs.

Firstly this PR moves typestubs frombuildconfig/pygame-stubs tobuildconfig/stubs/pygame because stubtest needs to run in a directory where pygame stubs are inpygame subdir.

Some of our util typestubs in_common has 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 withmypy after the changes

I added some missing stubs, some in_sdl2 and also stubs forbase andrwobject.

Another change I did was to properly typeVectorElementwiseProxy as a generic, so now it works well with bothVector2 andVector3 types, 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 replacedclock_init withclock_new following the PR#3001 becauseClock.__init__ leads to some stub issues, and also according to python docs fortp_new

A good rule of thumb is that for immutable types, all initialization should take place intp_new, while for mutable types, most initialization should be deferred totp_init.

So clock being immutable, this change makes sense

dr0id added a commit to dr0id/pygame that referenced this pull requestFeb 12, 2022
remove changes since this is addressed in PRpygame#2999
Copy link
Contributor

@dr0iddr0id left a 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.

@MyreMylar
Copy link
Contributor

Comparing this PR to 2.1.2 freetype module this now produces a typing error:

import pygamefont = pygame.freetype.Font(None, 32)

Is this an intentional change because of:

The pygame package does not import freetype automatically when loaded. This module must be imported explicitly to be used.import pygameimport pygame.freetype

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).

@ankith26
Copy link
ContributorAuthor

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.
The mixer fail is unrelated to this PR.

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

@MyreMylar
Copy link
Contributor

Sorry@ankith26 looks like the new vector functions have given you a conflict here

@ankith26
Copy link
ContributorAuthor

No worries, I'll resolve it with a rebase after zolds review is done (he's taking many days to review, yes 😆 )

Copy link
Contributor

@zoldalma999zoldalma999 left a 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.

Copy link
Contributor

@zoldalma999zoldalma999 left a 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! 🎉

@illumeillume added bug Code quality/robustnessCode quality and resilience to changes ciIssue with the Continuous Integration (CI), the actions/bots that test things labelsOct 29, 2022
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

3 more reviewers

@dr0iddr0iddr0id approved these changes

@MyreMylarMyreMylarMyreMylar approved these changes

@zoldalma999zoldalma999zoldalma999 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

bugciIssue with the Continuous Integration (CI), the actions/bots that test thingsCode quality/robustnessCode quality and resilience to changestypesType hint checking related tasks

Projects

None yet

Milestone

2.1.3

Development

Successfully merging this pull request may close these issues.

midi.Input.poll() listed as pool() in midi.pyi pygame.event.EventType lacks proper typing definition

6 participants

@ankith26@MyreMylar@dr0id@zoldalma999@illume

[8]ページ先頭

©2009-2025 Movatter.jp