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.

Sign up for GitHub

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

Expand __init__ parameters for Window #3295

Merged
merged 4 commits into from
Jan 21, 2025
Merged

Expand __init__ parameters for Window #3295

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
4182472
Expand init parameters for window
damusss Jan 17, 2025
90f1440
Add type ignore
damusss Jan 17, 2025
a1bd732
Remove the overload
damusss Jan 20, 2025
075d959
update return value hinting
damusss Jan 20, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 18 additions & 1 deletion buildconfig/stubs/pygame/window.pyi
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,24 @@ class Window:
title: str = "pygame window",
size: Point = (640, 480),
position: Union[int, Point] = WINDOWPOS_UNDEFINED,
**flags: bool
*,
fullscreen: bool = ...,
fullscreen_desktop: bool = ...,
opengl: bool = ...,
vulkan: bool = ...,
hidden: bool = ...,
borderless: bool = ...,
resizable: bool = ...,
minimized: bool = ...,
maximized: bool = ...,
mouse_grabbed: bool = ...,
keyboard_grabbed: bool = ...,
input_focus: bool = ...,
mouse_focus: bool = ...,
allow_high_dpi: bool = ...,
mouse_capture: bool = ...,
always_on_top: bool = ...,
utility: bool = ...,
) -> None: ...
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
) -> None: ...
**flags: bool,
) -> None: ...

Copy link
Member Author

Choose a reason for hiding this comment

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

Hi, why would that be necessary? any flag that is not in the current parameter list will result in an error. it seems to me the code you are suggesting doesn't reflect what's going on in C. if a new flag will be added, a new keyword parameter will be added too.

Copy link
Contributor

Choose a reason for hiding this comment

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

Okay. I was thinking about the original signature and assumed that it was supported.
If flags are to be deprecated though, will they be removed?

Copy link
Member Author

Choose a reason for hiding this comment

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

I thought about it too, like, "will this make it harder to remove things?" not really. what is in the docs is official and public. just because it wasn't in the stubs before doesn't mean it could just be removed. if a flag is deprecated, the keyword parameter will be removed. your linter will complain, but your code won't fail, it'll just output a warning. the linting failing is actually good, because it discourages usage, just like @deprecated does.

Copy link
Contributor

Choose a reason for hiding this comment

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

Disallowing deprecated parameters in typing is not appropriate.
Either, the deprecated parameters are still compatible with typing via the **kwargs parameter, or there is a deprecated overload for them (possibly using **kwargs). There might also be runtime warnings to accompany them.

Copy link
Member Author

Choose a reason for hiding this comment

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

the warning are sure. how would the overload be compatible with mypy? should it be added now? perhaps when deprecating the keyword argument will not be removed, but it could be typehinted in some special way, I hope so. Either way, I believe it to be slightly outside of scope of this PR. good for noticing tho :)

Copy link
Contributor

Choose a reason for hiding this comment

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

Sure.
When a flag is deprecated, we can either keep it in the signature or create a separate deprecated overload.

Thanks for clarifying this.

def destroy(self) -> None: ...
def set_windowed(self) -> None: ...
Expand Down
Loading