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

Move Window, display and system docs to stubs #3296

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

zoldalma999
Copy link
Member

Some progress on #2757

@zoldalma999 zoldalma999 requested a review from a team as a code owner January 17, 2025 21:24
@Starbuck5 Starbuck5 added the docs label Jan 19, 2025
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.

LGTM, thanks a lot for working on this! 🎉

Did a side by side comparison of the generated docs and the current docs on pyga.me, and found everything matching up nicely.

@Starbuck5
Copy link
Member

Doing a side by side is a good idea for reviewing, I did so myself as well.

In Window I see a few issues. The new generation shows most (but not all?) properties as methods.

image

It's Window.position, not Window.position().

Another thing about position is that previously it mentioned that it could be the CENTERED or UNDEFINED constants.

One other thing I noticed is that display.update doesn't show that it takes a list of rectangles in its signatures list:
image

Actually on more investigation it is the website that's out of date there, I told it to unpause its scheduled docs uploads.

@Starbuck5
Copy link
Member

There also is a merge conflict due to #3295

@bilhox
Copy link
Contributor

bilhox commented Jan 22, 2025

Before merging this PR, I'm more concerned about how the docstring is displayed by VSC:
image

The format doesn't really match, I thought maybe I need to install an extension for rst docstring, but then I saw numpy documentation :
image

@zoldalma999
Copy link
Member Author

The new generation shows most (but not all?) properties as methods.

Good catch, should be fixed now!

Another thing about position is that previously it mentioned that it could be the CENTERED or UNDEFINED constants.

That is because we could write anything there before. Right now return types are grabbed from the stubs. I added it to its description tho (which is where it belongs anyway in my opinion).

One other thing I noticed is that display.update doesn't show that it takes a list of rectangles in its signatures list

Again, stubs. The rectangle argument is typed so that it can be either one rectangle or a list of them. Could add another overload for it if you want me to (or change rectangle to rectangles)

Before merging this PR, I'm more concerned about how the docstring is displayed by VSC

They use their own style, which vsc (or rather, pylance) can render properly. RST is still a preview feature, and needs to be enabled manually.
image

@Starbuck5
Copy link
Member

Again, stubs. The rectangle argument is typed so that it can be either one rectangle or a list of them. Could add another overload for it if you want me to (or change rectangle to rectangles)

Probably another overload would do it. Right now the docs it generates are misleading because it just says "rectangle".

@Starbuck5
Copy link
Member

I still see weirdness with properties in Window.

image

All 4 of these are in Window.c getsets but half of them are seen one way and half of them are seen as functions.

@zoldalma999
Copy link
Member Author

I still see weirdness with properties in Window.

Maybe try doing a full generation? It does work for me.
image

Probably another overload would do it. Right now the docs it generates are misleading because it just says "rectangle".

Done!

I also fixed the warning that display.message_box threw about the window parameter. Now it should always link to the correct one.

Some things I noticed that could be worked on:

  • There's undocumented stuff related to window: pygame.window.get_grabbed_window and Window.relative_mouse
  • pygame.system.get_power_state has an experimental notice that it had for a while now. I think it can be removed at this point?
  • Maybe display.Info could be turned into a proper class with an init function, like we did with the rest of the classes. Also, add docstring for each attribute copied from the display.Info function.
  • Deprecate icontitle in pygame.display.set_caption - it does nothing and in pygame 3 change get_caption to only return a single string
  • display.get_num_displays references SDL 1 behavior

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants