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

Updated stub with imbedded documentation #236

Merged
merged 13 commits into from
Jun 5, 2021
Merged

Updated stub with imbedded documentation #236

merged 13 commits into from
Jun 5, 2021

Conversation

amoreaulemay
Copy link

Imbedded documentation in the stub, tested with pylance.

amoreaulemay added 2 commits May 31, 2021 15:53
Imbedded documentation in the stub, tested with pylance.
Uniformized the docstrings convention for consistency
@lazka
Copy link
Member

lazka commented Jun 3, 2021

Interesting :) But tests are failing

@naveen521kk
Copy link
Contributor

Will this fix #225?

But tests are failing

should be simple to make it pass :)

Hopefully this should past tests, I corrected forward references
@amoreaulemay
Copy link
Author

I updated the stub to correct for forward references, this should pass the test, it did locally for me

@amoreaulemay
Copy link
Author

I'm not too sure I understand the assertion error with typing

I copied the constants directly from the original __init__.pyi, where the tests passed. So It should pass now?
@amoreaulemay

This comment has been minimized.

I remodeled the file based on the original. The only major differences between the two are the docstrings now. If the original file passed the test, this should do as well.
@amoreaulemay
Copy link
Author

I remodeled the file based on the original. The only major differences between the two are the docstrings and the order of some functions within classes, which shouldn't influence the test. If the original file passed the test, this should do as well.

@amoreaulemay
Copy link
Author

Run all the Github actions for test on my side, passes all tests with the latest commit!

@lazka
Copy link
Member

lazka commented Jun 4, 2021

Thanks.

  1. Can you give some background on where you got the docs from and what's the syntax used?
  2. and can you answer Updated stub with imbedded documentation #236 (comment)

@amoreaulemay
Copy link
Author

Yes, so

  1. For the docs I took them from https://pycairo.readthedocs.io/en/latest/index.html, as for the format it's Numpydoc inspired but with some differences. I coded them that way because I'm using pylance, which for some reasons doesn't have good support for any docstring style. Here's an example on how it shows with pylance with the way I did it.
    ScreenShot Pylance
  2. To the best of my knowledge, it's not really a fix for Missing Docstrings #225 because the python interpreter doesn't really use stubs (or at least it doesn't for me). So the stub is mostly useful for two things: type checking and hinting in IDE. The previous stub already took care of the type checking, so this addition was just to get hints inside of IDEs, like with VScode with pylance.

@amoreaulemay
Copy link
Author

amoreaulemay commented Jun 5, 2021

Oh and to add more info on why I made the docstring that way is that pylance would only correctly display code snippets if preceded by >>>. Not a perfect solution, but seemed to work that way at least. Pylance is a relatively new tool, and I couldn't find any reliable documentation on the recommended docstrings format, so I mostly came up with that through trial and error. I did do my best to make it consistent across the stub however.

@lazka
Copy link
Member

lazka commented Jun 5, 2021

I see, thanks. It's a start, but we should try to avoid duplication so the best way forward is probably to use sphinx-autodoc with napoleon and hope pylance supports the standard docstring formats at some point.

@lazka lazka merged commit 2e8576b into pygobject:master Jun 5, 2021
@naveen521kk
Copy link
Contributor

pylance supports the standard docstring formats at some point.

Irrc, it's already supported?

@amoreaulemay amoreaulemay deleted the patch-1 branch June 5, 2021 18:44
@stuaxo
Copy link
Collaborator

stuaxo commented Jun 7, 2021

Realise this is merged already - but we should integrate linters for documentation too, to enforce whatever standards we decide on.

@lazka
Copy link
Member

lazka commented Feb 27, 2022

I've somewhat broken the vscode experience with this by switching back to rst in #252
The upside is that the website now uses the same source and we don't have to maintain the docstrings in two places.

I hope that someday vscode will figure out a way to parse reST, or at least adds some syntax highlighting: microsoft/pylance-release#2002

I hope you understand.

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

Successfully merging this pull request may close these issues.

4 participants