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

Added doc for Cursor object #2709

Merged
merged 12 commits into from Oct 23, 2021
Merged

Added doc for Cursor object #2709

merged 12 commits into from Oct 23, 2021

Conversation

fortwoone
Copy link
Contributor

The pygame.cursors.Cursor object is a brand new feature that I have just discovered, and I find sad that is is not documented already, so I tried to make a decent one

@Starbuck5
Copy link
Contributor

It is documented, it's just that the docs on the website haven't been updated since 2.0.0. If you generate the docs locally you'll be able to see it, or if you snoop around the .rst files themselves.

It is a shame though, so I wrote a reddit post that will hopefully explain it to people before the docs get regenerated: https://www.reddit.com/r/pygame/comments/octscc/definitive_guide_to_pygame_cursors/.

With that in mind, do we need docstrings in the source? Maybe the docstrings would should up on some editors, which could help people out? But they should probably be a close copy of what is in the documentation itself.

@fortwoone
Copy link
Contributor Author

Sure, but I noticed there was one init overload that was not documented, and I added it to the docstring inside the cursor object. It is not documented in the corresponding .rst file either sadly...

@Starbuck5
Copy link
Contributor

The Cursor(cursor) overload is arguably not important to document, since it's assumed behavior from most python objects.

But if you want to document it you should change it in the documentation, not in a docstring.

Also, your docstring misses an __init__ case, for the old school 4 argument cursors.

@fortwoone
Copy link
Contributor Author

Oh thanks for noticing. Do you think I should document it too ?

I also added a copy method in case someone wants to duplicate the cursor (even though I assume that would be very rare but still useful)
Matched the docstrings in cursors.py plus some extra info I noticed while reading the code
@fortwoone
Copy link
Contributor Author

Ok so now I am finished

src_py/cursors.py Outdated Show resolved Hide resolved
src_py/cursors.py Outdated Show resolved Hide resolved
src_py/cursors.py Outdated Show resolved Hide resolved
docs/reST/ref/cursors.rst Outdated Show resolved Hide resolved
@Starbuck5
Copy link
Contributor

Starbuck5 commented Oct 2, 2021

Also, it would be good if you could build the docs locally, because that would forward your changes to src_c/doc/cursors_doc.h

It should just be python setup.py docs, but if you're on an outdated version of main, it requires installing the pygame build dependencies, which is difficult on Windows. I actually just got a docs README merged, so you can check the docs folder on Github to see that.

@fortwoone
Copy link
Contributor Author

Ok I will check

Copy link
Member

@illume illume left a comment

Choose a reason for hiding this comment

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

Nice improvements to the Cursor docs, thanks.

These two things remain:

  • generate .h files (run python setup.py docs)
  • fix thoses typo which should be those.

Let us know if you want us to finish these two things off? We can merge your branch in, and then fix them.

@illume illume added cursors pygame.cursors docs labels Oct 16, 2021
@illume illume added this to the 2.0.3 milestone Oct 16, 2021
@fortwoone
Copy link
Contributor Author

Which setup.py should I run ? The one from my fork or the original one ?

@fortwoone
Copy link
Contributor Author

(also I corrected the typo)

@fortwoone
Copy link
Contributor Author

I corrected the typo myself, can you generate the .h files please ?

@illume
Copy link
Member

illume commented Oct 23, 2021

I corrected the typo myself, can you generate the .h files please ?

Cool, thanks. Will do.

@fortwoone
Copy link
Contributor Author

I corrected the typo myself, can you generate the .h files please ?

Cool, thanks. Will do.

Thanks a lot

@illume illume changed the base branch from main to fortwoone-docs October 23, 2021 15:58
@illume illume merged commit 41ce481 into pygame:fortwoone-docs Oct 23, 2021
@illume illume mentioned this pull request Oct 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cursors pygame.cursors docs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants