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

Correct pygame.display stubs #3264

Merged
merged 5 commits into from
Dec 19, 2024
Merged

Conversation

aatle
Copy link
Contributor

@aatle aatle commented Dec 15, 2024

pygame.display.get_surface() can return None if no display is initialized yet.
This correction may cause type checking errors in existing codebases.

pygame.display.update() can take floats for pixel coordinates, truncating them as usual.

@aatle aatle requested a review from a team as a code owner December 15, 2024 03:47
@yunline yunline added type hints Type hint checking related tasks display pygame.display labels Dec 15, 2024
Copy link
Member

@ankith26 ankith26 left a comment

Choose a reason for hiding this comment

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

Nice catch, thanks! 🎉

Left a minor review comment, but that won't stop my approval

buildconfig/stubs/pygame/display.pyi Show resolved Hide resolved
@damusss
Copy link
Member

damusss commented Dec 15, 2024

I'm slightly unsure about this. When I work on apps or games I don't enable type checkers, so this won't really affect me, but when I work on mili I do for people's sake and every time there is X|None I have to guard it at every usage. The reason I'm not sure it's because whether the function returns None doesn't depend on input or calculations but rather on the state of the program that a type checker can never know, meaning it'll always complain, even if you are sure the code path is going to run after display initialization.

I guess you can just blame this on the limitations of type checkers, and that the stubs mustn't lie to a user just because of it, and I can't complain on that, so I'll not stop this. I don't use this function either (window api on TOP), I was just rambling in the perspective on someone else (but at the same time, if they are good enough to use type checkers, they are good enough to use the window api lol)

@aatle
Copy link
Contributor Author

aatle commented Dec 15, 2024

Added an overload for no-argument display.update() to better reflect its behavior (differs from display.update(None)).
Updated docs, including listing more overloads for display.update().

@aatle
Copy link
Contributor Author

aatle commented Dec 15, 2024

I agree that it may inconvenience users who rely on this function to grab the display. They will probably have to use cast.
However, pygame.display.get_surface() is one of the main ways to check if the display is initialized, and it is important to correctly document its behavior. (Incorrect type annotations can lead to crashes when the type checker thought it was safe.)

Copy link
Member

@Starbuck5 Starbuck5 left a comment

Choose a reason for hiding this comment

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

Thanks!

@ankith26 ankith26 added this to the 2.5.3 milestone Dec 19, 2024
@ankith26 ankith26 merged commit 4b413fe into pygame-community:main Dec 19, 2024
25 checks passed
@aatle aatle deleted the display-stubs branch December 31, 2024 20:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
display pygame.display type hints Type hint checking related tasks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants